-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add volume prune --filter support #8689
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package filters | ||
|
||
import ( | ||
"net/url" | ||
"strings" | ||
|
||
"github.com/pkg/errors" | ||
) | ||
|
||
func ParseFilterArgumentsIntoFilters(filters []string) (url.Values, error) { | ||
parsedFilters := make(url.Values) | ||
for _, f := range filters { | ||
t := strings.SplitN(f, "=", 2) | ||
if len(t) < 2 { | ||
return parsedFilters, errors.Errorf("filter input must be in the form of filter=value: %s is invalid", f) | ||
} | ||
parsedFilters.Add(t[0], t[1]) | ||
} | ||
return parsedFilters, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -214,7 +214,7 @@ func (ic *ContainerEngine) SystemPrune(ctx context.Context, options entities.Sys | |
systemPruneReport.ImagePruneReport.Report.Id = append(systemPruneReport.ImagePruneReport.Report.Id, results...) | ||
} | ||
if options.Volume { | ||
volumePruneReport, err := ic.pruneVolumesHelper(ctx) | ||
volumePruneReport, err := ic.pruneVolumesHelper(ctx, nil) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be passing down the prune filters. podman system prune --filter should be passed down to volume helper as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that was my mistake. I can open a PR for that tonight. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
if err != nil { | ||
return nil, err | ||
} | ||
|
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 was conflicted whether to use
url.Values
or the underlying type ofmap[string][]string
for the filters. I saw it both ways throughout the codebase. I settled onurl.Values
but I don't feel strongly either way. I thought I remember seeing some mention of a refactor for filters so hopefully the inconsistency can be fixed then.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 feel like we should use
map[string][]string
everywhere.@baude @jwhonce WDYT?
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.
Once @baude / @jwhonce add their input I can fix the other things you mentioned @Luap99 (since the type will effect the changes you requested, even though its not much different).
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.
Just keep it for now. It will be reworked soon.