Skip to content

Conversation

@justinc1
Copy link
Collaborator

@justinc1 justinc1 commented May 8, 2023

The version_update_status_info uses undocumented API endpoint, it is hard to know what is returned. I believe we wanted to have ==COMPLETE, not ==COMPLETED.

I believe whole condition is there only to prevent starting second upgrade before the first one is finished -
.update_status != "IN PROGRESS" is sufficient.

A corner case would be if .update_status == SOME_ERROR or similar. In this case != "IN PROGRESS" would still allow starting an upgrade. The ==COMPLETE is again not needed - better to remove it, it just adds to confusion.

The version_update_status_info uses undocumented API endpoint,
it is hard to know what is returned. I believe we wanted to have
`==COMPLETE`, not `==COMPLETED`.

I believe whole condition is there only to prevent starting
second upgrade before the first one is finished -
`.update_status != "IN PROGRESS"` is sufficient.

A corner case would be if `.update_status == SOME_ERROR` or similar.
In this case `!= "IN PROGRESS"` would still allow starting an upgrade.
The `==COMPLETE` is again not needed - better to remove it,
it just adds to confusion.

Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
@justinc1 justinc1 requested a review from anazobec May 8, 2023 08:51
@justinc1 justinc1 self-assigned this May 8, 2023
@justinc1
Copy link
Collaborator Author

justinc1 commented May 8, 2023

A stupid question for @PolonaM - the update_status_version_info module should never return COMPLETED in .update_status field? It is not even relevant for this PR, but it would be a nightmare if we would sometimes have COMPLETE, and sometimes COMPLETED.

@PolonaM
Copy link
Collaborator

PolonaM commented May 8, 2023

Yes, it should never be COMPLETED, at least from what I saw, It is always COMPLETE.

Copy link
Collaborator

@anazobec anazobec 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. Approved.

@anazobec anazobec merged commit ded8dba into main May 8, 2023
@anazobec anazobec deleted the cleanup-update-status-check branch May 8, 2023 10:00
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.

4 participants