Skip to content
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

Make 'chainloop workflow create --name build --project xy --team abc' idempotent #1206

Closed
habnux opened this issue Aug 12, 2024 · 7 comments · Fixed by #1214
Closed

Make 'chainloop workflow create --name build --project xy --team abc' idempotent #1206

habnux opened this issue Aug 12, 2024 · 7 comments · Fixed by #1214

Comments

@habnux
Copy link

habnux commented Aug 12, 2024

Today, the CLI returns RC=1 if the workflow already exists for the specified name and project. In the interest of automation, it makes sense that this does not fail. However, a warning could still be displayed.

chainloop workflow create --name build --project xy --team abc
ERR failed to create workflow: rpc error: code = InvalidArgument desc = validation error: name already taken
echo $?
1
@migmartri migmartri self-assigned this Aug 12, 2024
@migmartri
Copy link
Member

migmartri commented Aug 12, 2024

Thanks, @habnux, for reporting this issue. To me, we have three options here, taking into account that there is an additional implicit requirement to differentiate when there is an actual error vs knowing that there is already a workflow, which is also represented as an error :)

a) return different error codes. This is basically what we have today but the user will be able to differentiate if the command actually failed.
b) have a flag that makes the command succeed if the workflow exists. i.e --skip-if-exists,
c) make the idempotent feature default, so no flag is needed.

I am leaning towards option b since a doesn't offer any improvement in user experience, and c changes a default behavior that's different from all the other commands. To me, b is a good compromise, and the user who's implementing automation can choose to use it as such.

What do you think @habnux, @jiparis?

@migmartri
Copy link
Member

This feature will also help here #961

@jiparis
Copy link
Member

jiparis commented Aug 12, 2024

I'm wondering if there are other commands (other "create") commands, where we should also enable such flag. Maybe some generic flag for "recoverable" errors (--ignore-errors)

@habnux
Copy link
Author

habnux commented Aug 12, 2024

I'm wondering if there are other commands (other "create") commands, where we should also enable such flag. Maybe some generic flag for "recoverable" errors (--ignore-errors)

Imo, ignoring errors is never a good choice ... The creation of a clear and comprehensible interface is crucial.

@migmartri
Copy link
Member

@habnux so would option b work for you? #1206 (comment)

offer a specific flag --skip-if-exists that will return exit code 0 if the entity already exists?

@habnux
Copy link
Author

habnux commented Aug 13, 2024

Yes option B is okey. Another idea is to use the chainloop workflow update ... with something like --create in case a workflow does not already exist, it will be created. But that's probably more a question of taste.

@migmartri
Copy link
Member

migmartri commented Aug 13, 2024

Thanks, @habnux. I think your proposal will require more changes since creation and update might have different required arguments. i.e we will need to make project mandatory on update if we want to make sure we have a valid workflow created after the process.

In any case, we want to support chainloop apply in the future, though, where you provide, for example, a workflow resource that will create or update a workflow similar to kubectl apply in k8s. Would that work for you?

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

Successfully merging a pull request may close this issue.

3 participants