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

119 fix lifecycle/configure.py's defaults/arguments functions, does not work for @click.argument types #127

Conversation

chrisfandrade16
Copy link
Contributor

@chrisfandrade16 chrisfandrade16 commented Sep 12, 2024

Since we are using the .main method to call a Click command (as opposed to .callback which skips the Click CLI), all defaults are already calculated. As such, we don't even need to use configure.defaults. Instead, I modified configure.arguments to correctly build the Click CLI command for the .main method usage, such that the Click CLI is used and all defaults, type conversions, and constraints and performed automatically by Click. This new modified configure.arguments function accounts for both @click.argument and @click.option types, as well as is_flag=True types.

@odarotto
Copy link
Collaborator

@chrisfandrade16 LGMT, I think this would fix #97, have you tested the case were the Work object is retried and keeps the same parameters?

@chrisfandrade16
Copy link
Contributor Author

@odarotto Good idea, I have not tried that...any ideas on how to test that? A function that fails the first time but works the second? Perhaps I can think of something.

@chrisfandrade16
Copy link
Contributor Author

@odarotto Just tested it. Ran a function that fails the first time (checks for a file, if does not exist, fails but creates it so the next time it succeeds), with work.retries = 2. Tried it with both @click.argument and @click.option and is_flag=True, all the parameters were still there the second time. Honestly, I'm not sure what I did to fix that issue though haha. I don't know what configure.defaults was doing in the first place to cause that.

@chrisfandrade16 chrisfandrade16 merged commit 196d8fe into main Sep 13, 2024
3 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FIX] lifecycle/configure.py's "defaults"/"arguments" function does not work for @click.argument types
2 participants