-
Notifications
You must be signed in to change notification settings - Fork 1.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
allow duration based filters on diskusage requests #5455
base: master
Are you sure you want to change the base?
Conversation
Allows similar time-based filter that is allowed for prune requests so that DiskUsage request can be used to check which records would be candidates for pruning. Signed-off-by: Tonis Tiigi <[email protected]>
@@ -36,6 +36,7 @@ message PruneRequest { | |||
|
|||
message DiskUsageRequest { | |||
repeated string filter = 1; | |||
int64 ageLimit = 2; |
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.
Struggled to find a good name for this. In Prune it is called KeepDuration
but that name does not make sense at all for DiskUsage.
In buildx this is --filter until=<duration>
that is detected on client side and converted to another field. buildx du
will likely keep the same name there.
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.
ageLimit
SGTM - but if we change this, would it also make sense to change the name in the prune request? We're already making changes to these structs in the next release because of the new storage fields.
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.
if prune had ageLimit
is it obvious which way it works? Eg. is it pruning the things that are newer than ageLimit
or older? Correct is older but I don't know if it is obvious.
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 second this, ageLimit
is fine for disk usage request. WDYT @dvdksn?
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.
Alternatively maxAge
, we use maxUsedSpace
in GC policy.
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.
maxAge
is nice, it works well in both places 🎉
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.
If anything, I think this is minAge
Allows similar time-based filter that is allowed for prune requests so that DiskUsage request can be used to check which records would be candidates for pruning.