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

CLI: don't override flags for a release when editing it #4484

Merged

Conversation

AdamWill
Copy link
Contributor

Because the defaults in release_options for composed_by_bodhi and
create_automatic_updates were set to True and False respectively,
and edit_release uses those options, when you edit a release -
even if you don't specifically pass any argument to set either of
those flags - the CLI will change whatever the current values of
those flags are for the release you edit to those defaults. So
if you edit a release with composed_by_bodhi set to False and
don't pass --composed-by-bodhi or --not-composed-by-bodhi,
the flag will be changed to True.

This solves that by changing the defaults to None - which causes
edit_release to properly leave them at their current values
unless the user specifically passed a relevant argument - and
having create_release set them to the expected default value
if they come in as None.

We also throw away the explicit handling of composed_by_bodhi
that was added to both functions in 3de3588, because it is
unnecessary. It is already coming into both functions as a kwarg;
handling it like this changes nothing at all. Just adding the
option to release_options was all that was ever needed.

Signed-off-by: Adam Williamson awilliam@redhat.com

Because the defaults in release_options for composed_by_bodhi and
create_automatic_updates were set to True and False respectively,
and `edit_release` uses those options, when you edit a release -
even if you don't specifically pass any argument to set either of
those flags - the CLI will change whatever the current values of
those flags are for the release you edit to those defaults. So
if you edit a release with composed_by_bodhi set to False and
don't pass `--composed-by-bodhi` or `--not-composed-by-bodhi`,
the flag will be changed to True.

This solves that by changing the defaults to None - which causes
`edit_release` to properly leave them at their current values
unless the user specifically passed a relevant argument - and
having `create_release` set them to the expected default value
if they come in as None.

We also throw away the explicit handling of composed_by_bodhi
that was added to both functions in 3de3588, because it is
unnecessary. It is already coming into both functions as a kwarg;
handling it like this changes nothing at all. Just adding the
option to release_options was all that was ever needed.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
@AdamWill AdamWill requested a review from a team as a code owner April 25, 2022 22:47
@AdamWill
Copy link
Contributor Author

We ran into this problem today due to https://pagure.io/releng/issue/10761 - when @humaton ran the command to change the branch name to "rawhide" as requested there, it changed composed_by_bodhi to True (it should be False for Rawhide) and create_automatic_updates to False (it should be True for Rawhide). This broke Rawhide update creation for a few hours and left us with a bit of a mess to clean up, so it's a significant bug.

@AdamWill
Copy link
Contributor Author

Awesome job @humaton for posting the output of the command to the ticket, BTW - this would've been a lot harder to debug without it!

@AdamWill
Copy link
Contributor Author

Note, I tested this by short-circuiting both functions to just print the data they would submit and then exit (instead of submitting it). It looks to me like it works as expected in all cases.

@nirik
Copy link
Member

nirik commented Apr 25, 2022

Looks completely reasonable to me. +1

@humaton humaton merged commit 9351687 into fedora-infra:develop May 3, 2022
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.

None yet

3 participants