Skip to content
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.

Sign up for GitHub

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

feat(cli): archive list option to filter by name prefix. Fixes #10524 #13794

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat(cli): archive list option to filter by name prefix. Fixes #10524 #13794

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
60f5a6a
Signed-off-by: tooptoop4 <[email protected]>
tooptoop4 Oct 20, 2024
8045fcc
Signed-off-by: tooptoop4 <[email protected]>
tooptoop4 Oct 20, 2024
eeec127
Signed-off-by: tooptoop4 <[email protected]>
tooptoop4 Oct 20, 2024
e9bc778
Signed-off-by: tooptoop4 <[email protected]>
tooptoop4 Oct 20, 2024
6125f6f
Signed-off-by: tooptoop4 <[email protected]>
tooptoop4 Oct 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmd/argo/commands/archive/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func NewListCommand() *cobra.Command {
selector string
output = common.NewPrintWorkflowOutputValue("wide")
chunkSize int64
prefix string
)
command := &cobra.Command{
Use: "list",
Expand All @@ -47,28 +48,29 @@ func NewListCommand() *cobra.Command {
return err
}
namespace := client.Namespace()
workflows, err := listArchivedWorkflows(ctx, serviceClient, namespace, selector, chunkSize)
workflows, err := listArchivedWorkflows(ctx, serviceClient, namespace, selector, chunkSize, prefix)
if err != nil {
return err
}
return printer.PrintWorkflows(workflows, os.Stdout, printer.PrintOpts{Output: output.String(), Namespace: true, UID: true})
},
}
command.Flags().VarP(&output, "output", "o", "Output format. "+output.Usage())
command.Flags().StringVar(&prefix, "prefix", "", "Filter workflows by prefix")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should be more specific like "name prefix"; the API parameter is NamePrefix too, so it would be good to match

Suggested change
command.Flags().StringVar(&prefix, "prefix", "", "Filter workflows by prefix")
command.Flags().StringVar(&prefix, "name-prefix", "", "Filter workflows by name prefix")

Copy link

@agilgur5 agilgur5 Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also make sense to consider the UX/style of #13160, where --name can be modified by --name-filter, which is one of "Exact" / "Prefix" / "Pattern".

Although it didn't implement that for the Archived List API specifically, but I think that API is capable of both "Exact" and "Prefix" already, so we can make the CLI syntax/UX match the newer List API

Copy link

@agilgur5 agilgur5 Oct 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see --prefix and the description verbatim match the list CLI, so perhaps we leave that in for backward-compat but also add --name and --name-filter to match #13160.

We could also remove it in a breaking CLI change or just only add the new variant to the archive list command so that there's nothing to deprecate later.

command.Flags().StringVarP(&selector, "selector", "l", "", "Selector (label query) to filter on, not including uninitialized ones, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)")
command.Flags().Int64VarP(&chunkSize, "chunk-size", "", 0, "Return large lists in chunks rather than all at once. Pass 0 to disable.")
return command
}

func listArchivedWorkflows(ctx context.Context, serviceClient workflowarchivepkg.ArchivedWorkflowServiceClient, namespace string, labelSelector string, chunkSize int64) (wfv1.Workflows, error) {
func listArchivedWorkflows(ctx context.Context, serviceClient workflowarchivepkg.ArchivedWorkflowServiceClient, namespace string, labelSelector string, chunkSize int64, prefix string) (wfv1.Workflows, error) {
listOpts := &metav1.ListOptions{
LabelSelector: labelSelector,
Limit: chunkSize,
}
var workflows wfv1.Workflows
for {
log.WithField("listOpts", listOpts).Debug()
resp, err := serviceClient.ListArchivedWorkflows(ctx, &workflowarchivepkg.ListArchivedWorkflowsRequest{Namespace: namespace, ListOptions: listOpts})
resp, err := serviceClient.ListArchivedWorkflows(ctx, &workflowarchivepkg.ListArchivedWorkflowsRequest{Namespace: namespace, ListOptions: listOpts, NamePrefix: prefix})
if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/argo/commands/archive/resubmit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func resubmitArchivedWorkflows(ctx context.Context, archiveServiceClient workflo
)

if resubmitOpts.hasSelector() {
wfs, err = listArchivedWorkflows(ctx, archiveServiceClient, resubmitOpts.fieldSelector, resubmitOpts.labelSelector, 0)
wfs, err = listArchivedWorkflows(ctx, archiveServiceClient, resubmitOpts.fieldSelector, resubmitOpts.labelSelector, 0, "")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/argo/commands/archive/retry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func retryArchivedWorkflows(ctx context.Context, archiveServiceClient workflowar
}
var wfs wfv1.Workflows
if retryOpts.hasSelector() {
wfs, err = listArchivedWorkflows(ctx, archiveServiceClient, retryOpts.fieldSelector, retryOpts.labelSelector, 0)
wfs, err = listArchivedWorkflows(ctx, archiveServiceClient, retryOpts.fieldSelector, retryOpts.labelSelector, 0, "")
if err != nil {
return err
}
Expand Down
1 change: 1 addition & 0 deletions docs/cli/argo_archive_list.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ argo archive list [flags]
--chunk-size int Return large lists in chunks rather than all at once. Pass 0 to disable.
-h, --help help for list
-o, --output string Output format. One of: name|json|yaml|wide (default "wide")
--prefix string Filter workflows by prefix
-l, --selector string Selector (label query) to filter on, not including uninitialized ones, supports '=', '==', and '!='.(e.g. -l key1=value1,key2=value2)
```

Expand Down
Loading