Skip to content

feat: Added retention command#184

Closed
Althaf66 wants to merge 2 commits into
goharbor:mainfrom
Althaf66:Althaf66/retentioncmd
Closed

feat: Added retention command#184
Althaf66 wants to merge 2 commits into
goharbor:mainfrom
Althaf66:Althaf66/retentioncmd

Conversation

@Althaf66

@Althaf66 Althaf66 commented Aug 25, 2024

Copy link
Copy Markdown
Contributor

Added new retention commands

harbor retention create - create retention policy for the project
harbor retention list - list retention execution for the project
harbor retention delete - delete retention policy for the project

this PR fixes part of #94

Signed-off-by: ALTHAF <althafasharaf02@gmail.com>
@Vad1mo Vad1mo requested a review from ambahman August 27, 2024 12:42
Signed-off-by: ALTHAF <althafasharaf02@gmail.com>
@bupd bupd self-requested a review November 30, 2024 00:12
@Jellyfrog

Copy link
Copy Markdown

Is there anything preventing this from being merged? (Except for the merge conflict)

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall, Retention should be a subcommand of project or tag.

I believe it is better to have it as subcommand for tag. Since we also have tag immutability rules.

Example use case:

./harbor tag retention --projectname ...flags
./harbor tag immutable --projectname ...flags

The above would make much sense from a user perspective. Which is also clear to the user.

cmd := &cobra.Command{
Use: "retention",
Short: "Manage retention rule in the project",
Long: `Manage retention rules in the project in Harbor`,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can use the Long explanation to explain what does this command do.
Since retention is a TAG RETENTION policy, we should communicate this well to the user.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also mention, "user can only create 15 tag retention rules per project".

Use: "retention",
Short: "Manage retention rule in the project",
Long: `Manage retention rules in the project in Harbor`,
Example: `harbor retention create`,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some example usecases here would be beneficial.

func ListExecutionRetentionCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "list",
Short: "list retention execution of the project",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is an absolute wrong approach here.

As an user I would expect

./harbor project retention list

to return me the list of retention rules that I made. Instead of retention executions.

FIX THIS.

Comment on lines +19 to +20
Use: "list",
Short: "list retention execution of the project",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is absolute wrong approach here.

As an user I would expect

./harbor tag retention list

to return me the list of retention rules that I made. Instead of retention executions.

FIX THIS.

@bupd bupd linked an issue Dec 30, 2024 that may be closed by this pull request
@bupd bupd added the help wanted Extra attention is needed label Mar 15, 2025
@rizul2108

Copy link
Copy Markdown
Collaborator

@Althaf66 Will you work further on this issue or should I continue from here?

@Althaf66

Copy link
Copy Markdown
Contributor Author

You can work on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement retention command

5 participants