feat: added exclude filter for purge command #428
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sashokbg
requested review from
sajayantony,
Wwwsylvia,
shizhMSFT,
northtyphoon,
wangxiaoxuan273,
wju-MSFT and
estebanreyl
as code owners
sashokbg
force-pushed
the
feat/added_purge_exclude_filter
branch
from
a0b56e1 to
b91358e
Compare
estebanreyl
requested changes
May 21, 2025
| @@ -355,11 +396,21 @@ func dryRunPurge(ctx context.Context, acrClient api.AcrCLIClientInterface, login | |||
| return -1, -1, err | |||
| } | |||
|
|
|||
| var excludeRegex *regexp2.Regexp = nil | |||
There was a problem hiding this comment.
Nit. Assigning nil to a pointer is unnecessary
| @@ -127,6 +127,18 @@ Examples of filters | |||
| | Untag all tags that are older than the duration in app repository | --filter `"app:.*"` | | |||
| | Untag all tags that are older than the duration in all repositories | --filter `".*:.*"` | | |||
|
|
|||
| ##### Exclude filter tag | |||
There was a problem hiding this comment.
I think we can keep this at 4#
| @@ -127,6 +127,18 @@ Examples of filters | |||
| | Untag all tags that are older than the duration in app repository | --filter `"app:.*"` | | |||
| | Untag all tags that are older than the duration in all repositories | --filter `".*:.*"` | | |||
|
|
|||
| ##### Exclude filter tag | |||
|
|
|||
| To exclude tags from the purging, use the --exclude-filter parameter. Exclude filters take precedence over filters. | |||
There was a problem hiding this comment.
Can you note that these are optional?
| if excludeFilter != "" { | ||
| res, err := common.BuildRegexFilter(excludeFilter, regexMatchTimeout) | ||
| if err != nil { | ||
| return -1, -1, err |
There was a problem hiding this comment.
Same as above, can you add the error wrap?
| var excludeRegex *regexp2.Regexp = nil | ||
|
|
||
| if excludeFilter != "" { | ||
| res, err := common.BuildRegexFilter(excludeFilter, regexpMatchTimeoutSeconds) |
There was a problem hiding this comment.
Can you add an error wrap, just to make it clearer? Like fmt.Errorf("could not build excludeFilter regex: %w:" same for the actual filter since this change will make it a little unclear as to which failed to build.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of the PR
Adds a new parameter "--exclude-filter" that will override any matches in the "--filter".
Useful when we want to purge everything except some tags (ex: dev, prod, release etc)
Fixes #
#60 #59