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

feat(cli): option to skip metadata update #3025

Merged
merged 11 commits into from Jul 19, 2022

Conversation

olevski
Copy link
Member

@olevski olevski commented Jul 15, 2022

This is a feature that can be useful in cases where the metadata update can happen elsewhere (i.e. argo workflow) or simply should not be committed.

For example running tests or troubleshooting things should not be tracked.

I was not sure where to exactly put the logic. I can move it up or down the chain of depedencies. I.e. we can have a whole new function called run_without_metadata_update instead of passing the skip metadata flag in the current function.

closes #3020

@olevski olevski requested a review from a team as a code owner July 15, 2022 15:22
@olevski olevski force-pushed the feature/3020-skip-metadata-update branch from 9e1978b to 7d2645d Compare July 17, 2022 21:11
Copy link
Member

@Panaetius Panaetius left a comment

Choose a reason for hiding this comment

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

This is nice, thank you!

We should probably add it for other execution commands as well (renku update/renku rerun/renku workflow iterate)

renku/command/run.py Outdated Show resolved Hide resolved
@Panaetius
Copy link
Member

oh also, documentation of the parameter in the docstring of the files at the top of the renku.ui.cli.* files and we maybe should add a shorthand flag for the --skip-metadata-update flag (-s?) as it's a bit of a mouthful to write

@olevski
Copy link
Member Author

olevski commented Jul 18, 2022

oh also, documentation of the parameter in the docstring of the files at the top of the renku.ui.cli.* files and we maybe should add a shorthand flag for the --skip-metadata-update flag (-s?) as it's a bit of a mouthful to write

Done you can use -s or --skip-metadata-update.

@olevski
Copy link
Member Author

olevski commented Jul 18, 2022

I also added the -s option to:

  • rerun
  • update
  • workflow iterate

And I removed the -s option from run as requested.

Panaetius
Panaetius previously approved these changes Jul 18, 2022
Copy link
Member

@Panaetius Panaetius left a comment

Choose a reason for hiding this comment

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

Thank you!

@olevski
Copy link
Member Author

olevski commented Jul 18, 2022

@Panaetius I am sorry for making you re-approve this. I had some issues with the style checks. My pre-commit hooks are broken it seems. Or that specific style check is not enforced by the pre-commit tests. Anyhow now the style check passes.

@olevski
Copy link
Member Author

olevski commented Jul 18, 2022

Ok the problem is that -s is taken for --set in workflow execute and this was breaking the toil workflow execute tests in the CI. So I will just use the long version of the flag for this. I guess I can try a different letter that is not taken but if it is not -s I think it is just confusing. I guess I can shorten the --skip-metadata-update but I think clarity is better than saving on typing 4-5 characters.

@olevski olevski force-pushed the feature/3020-skip-metadata-update branch from 6dd0e2e to 3616b70 Compare July 18, 2022 21:04
@olevski olevski force-pushed the feature/3020-skip-metadata-update branch from 3616b70 to 7496d24 Compare July 18, 2022 22:00
Panaetius
Panaetius previously approved these changes Jul 19, 2022
@olevski olevski enabled auto-merge (squash) July 19, 2022 11:03
@olevski olevski merged commit c89aba7 into develop Jul 19, 2022
@olevski olevski deleted the feature/3020-skip-metadata-update branch July 19, 2022 13:08
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.

Skip metadata update and commit in renku run and workflow execute
2 participants