-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(cli): Migrate argo wait
to use API client. See #2116
#2282
Conversation
argo watch
to use API client. See #2116argo wait
to use API client. See #2116
Codecov Report
@@ Coverage Diff @@
## master #2282 +/- ##
==========================================
+ Coverage 11.46% 13.04% +1.58%
==========================================
Files 71 70 -1
Lines 27791 24431 -3360
==========================================
+ Hits 3187 3188 +1
+ Misses 24199 20838 -3361
Partials 405 405
Continue to review full report at Codecov.
|
@@ -43,7 +45,7 @@ func newArgoKubeClient(clientConfig clientcmd.ClientConfig) (context.Context, Cl | |||
} | |||
|
|||
func (a *argoKubeClient) NewWorkflowServiceClient() workflowpkg.WorkflowServiceClient { | |||
return &argoKubeWorkflowServiceClient{workflowserver.NewWorkflowServer(sqldb.ExplosiveOffloadNodeStatusRepo)} | |||
return &argoKubeWorkflowServiceClient{workflowserver.NewWorkflowServer(argoKubeOffloadNodeStatusRepo)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see the benefit of this change. are you using the same variable anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in another PR I think - I did wonder if we should actually just log the error rather than explode- thoughts
ok, so repro using |
Checklist:
"fix(controller): Updates such and such. Fixes #1234"
.See #2116.
Please merge this as soon as builds are green and it is approved.