refactor(cli): Introduce v1.Interface for CLI. Closes #2107 #2048
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2048 +/- ##
=========================================
Coverage ? 11.69%
=========================================
Files ? 52
Lines ? 26337
Branches ? 0
=========================================
Hits ? 3081
Misses ? 22861
Partials ? 395
Continue to review full report at Codecov.
|
alexec
changed the title
| clientset *kubernetes.Clientset | ||
| wfClientset *versioned.Clientset | ||
| wfClient v1alpha1.WorkflowInterface | ||
| // DEPRECATED |
There was a problem hiding this comment.
Why can't these just be deleted? AFAIK, people don't import our CLI
There was a problem hiding this comment.
our CLI uses these methods - this is really more to make it easy for us to identify methods to remove
| return newArgoServerClient(argoServer, token) | ||
| } else { | ||
| return newClassicClient(clientConfig) | ||
| } |
| @@ -251,9 +244,9 @@ func (s *E2ESuite) printPodLogs(logCtx *log.Entry, namespace, pod, container str | |||
| fmt.Println("---") | |||
| } | |||
|
|
|||
| func (s *E2ESuite) Given() *Given { | |||
| func (s *E2ESuite) Given(t *testing.T) *Given { | |||
There was a problem hiding this comment.
to correctly report errors to sub-tests
Co-Authored-By: Simon Behar <simbeh7@gmail.com>
Co-Authored-By: Simon Behar <simbeh7@gmail.com>
Co-Authored-By: Simon Behar <simbeh7@gmail.com>
| @@ -40,7 +40,7 @@ func NewResubmitCommand() *cobra.Command { | |||
| defer conn.Close() | |||
| apiGRPCClient, ctx := GetWFApiServerGRPCClient(conn) | |||
| errors.CheckError(err) | |||
| wfReq := workflow.WorkflowGetRequest{ | |||
| wfReq := workflowpkg.WorkflowGetRequest{ | |||
| Namespace: namespace, | |||
| Name: args[0], | |||
| } | |||
There was a problem hiding this comment.
The rest of the implementation still remains the old way of doing. Can it not be refactored?
There was a problem hiding this comment.
I'd like to do it piecemeal - as a lot of the CLI code has no automated test - so refactoring is more likely to introduce bugs and regressions.
Whenever we need to change the CLI, we should complete the appropriate part of the client and migrate.
| @@ -26,7 +26,7 @@ func NewResumeCommand() *cobra.Command { | |||
| conn := client.GetClientConn() | |||
| @@ -38,7 +38,7 @@ func apiServerWatchWorkflow(wfName string) { | |||
| defer conn.Close() | |||
| apiClient, ctx := GetWFApiServerGRPCClient(conn) | |||
| fieldSelector := fields.ParseSelectorOrDie(fmt.Sprintf("metadata.name=%s", wfName)) | |||
| wfReq := workflow.WatchWorkflowsRequest{ | |||
| wfReq := workflowpkg.WatchWorkflowsRequest{ | |||
| Namespace: namespace, | |||
| ListOptions: &metav1.ListOptions{ | |||
| FieldSelector: fieldSelector.String(), | |||
There was a problem hiding this comment.
Same as the previous comment
alexec
changed the title
Checklist:
"fix(controller): Updates such and such. Fixes #1234".See #1951
Closes #2107
Anyone reviewing this - we should sit down so we can talk over the solution.