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

Fix snap promotion #240

Merged
merged 5 commits into from
Jan 9, 2025
Merged

Fix snap promotion #240

merged 5 commits into from
Jan 9, 2025

Conversation

omar-selo
Copy link
Collaborator

Description

When a snap is available in more than one risk at a time (e.g. edge and beta) TO sets it's stage to the first risk it sees instead of the highest one. This PR fixes the bug.

Resolved issues

Fixes https://warthogs.atlassian.net/browse/RTW-426

Documentation

Web service API changes

Tests

Added bug as a test case.

@omar-selo omar-selo requested a review from nadzyah January 9, 2025 14:03
Copy link
Contributor

@plars plars left a comment

Choose a reason for hiding this comment

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

One optional comment below, feel free to ignore though as it's just a subjective thing.
Thanks for the quick fix on this! It's interesting that even before the fix, it seems to eventually make it to the correct channel in TO though.

risk: str
risk: Literal[StageName.edge] | Literal[StageName.beta] | Literal[
StageName.candidate
] | Literal[StageName.stable]
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor/optional comment, I think you can also specify this as something like:
Literal[StageName.edge, StageName.beta, StageName.candidate, StageName.stable]
Or even set something like:

ValidSnapStages = Literal[StageName.edge, StageName.beta, StageName.candidate, StageName.stable]
...
risk: ValidSnapStages
...

Copy link
Collaborator Author

@omar-selo omar-selo Jan 9, 2025

Choose a reason for hiding this comment

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

Oh I didn't know that Literal could take multiple values. I was thinking of using the second approach but I'll delay it to my next PR that should include images as otherwise there will be some merge conflicts

@omar-selo omar-selo merged commit 9c9a97a into main Jan 9, 2025
1 check passed
@omar-selo omar-selo deleted the fix-snap-promotion branch January 9, 2025 14:30
@omar-selo
Copy link
Collaborator Author

it seems to eventually make it to the correct channel in TO though.

Yeah that happens because typically a newer version pops up in the risk it was previously at. So TO would grab the next available channel map that is in a higher risk

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.

2 participants