Skip to content

feat: require project name on workflow management #1378

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

Merged
merged 5 commits into from
Oct 10, 2024

Conversation

migmartri
Copy link
Member

@migmartri migmartri commented Oct 9, 2024

This is the first step towards #1375 it

  • Updates management APIs that reference workflows by their name to also require their project name.
  • Updates the attestation init with an optional --project flag so we can cut a release and prepare our clients (and dagger module) for a switch.

@migmartri migmartri requested review from jiparis and javirln October 9, 2024 15:38
if workflowName == "" {
return errors.New("workflow name is required, set it via --name flag")
return errors.New("workflow name is required, set it via --workflow flag")
Copy link
Member Author

Choose a reason for hiding this comment

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

I got some feedback from @juamedgod that --name was confusing since it seemed the name of the attestation itself.

So I have changed it to workflow while keeping the existing --name as deprecated

cmd.Flags().StringVar(&workflowName, "name", "", "name of the workflow to run the attestation")

cobra.CheckErr(cmd.Flags().MarkDeprecated("name", "please use --workflow instead"))
cmd.Flags().StringVar(&projectName, "project", "", "name of the project of this workflow")
Copy link
Member Author

Choose a reason for hiding this comment

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

project is not yet fully enforced in the attestation init

@@ -38,5 +38,8 @@ func newWorkflowDeleteCmd() *cobra.Command {
cmd.Flags().StringVar(&name, "name", "", "workflow name")
cobra.CheckErr(cmd.MarkFlagRequired("name"))

cmd.Flags().StringVar(&projectName, "project", "", "project name")
cobra.CheckErr(cmd.MarkFlagRequired("project"))
Copy link
Member Author

Choose a reason for hiding this comment

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

but it is in the rest of the APIs.

Copy link
Member

@javirln javirln left a comment

Choose a reason for hiding this comment

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

LGTM!

@migmartri migmartri merged commit 4978e66 into chainloop-dev:main Oct 10, 2024
13 checks passed
@migmartri migmartri deleted the 1375-warn-user branch October 10, 2024 09:45
gt.AppendRow(table.Row{"Name", meta.Name})
gt.AppendRow(table.Row{"Team", meta.Team})
Copy link
Member

Choose a reason for hiding this comment

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

Are we removing team in the future?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, but I see no value in showing it, not sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants