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

SG-36291 Do not downgrade startup versions #131

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

julien-lang
Copy link
Contributor

@julien-lang julien-lang commented Sep 18, 2024

The desktop-startup self-update itself. It downloads the new version from app store, installs it and restarts itself with the new version.

When communicating with the app_store, it will select the latest available version. Even if the version is older than itself.
So in the rare case where the local startup version was up-to-date yesterday but was marked "bad" on app store, startup will downgrade to the previous version.

At the same time, the desktop process will always select the latest startup between the bundled version and the downloaded version. So we get into a restart loop because these 2 algorithms don't have an aligned behaviour.

This PR aligns the behavior but also creates an environment variable to allow older startup version to be downloaded and used.

@julien-lang julien-lang requested a review from a team September 18, 2024 21:30
Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

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

I like this. Thanks!
Please add some description to the PR.

Comment on lines +75 to +85
if current_version <= app_store_version:
pass
elif os.environ.get("SGTK_STARTUP_ALLOW_OLDER_VERSION"):
pass
else:
logger.warning(
"Ignore app_store version %s since current version is newer %s",
latest_descriptor.version,
current_desc.get_version(),
)
return False
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't tested it but can it be rewritten like this?

Suggested change
if current_version <= app_store_version:
pass
elif os.environ.get("SGTK_STARTUP_ALLOW_OLDER_VERSION"):
pass
else:
logger.warning(
"Ignore app_store version %s since current version is newer %s",
latest_descriptor.version,
current_desc.get_version(),
)
return False
if os.environ.get("SGTK_STARTUP_ALLOW_OLDER_VERSION"):
pass
elif current_version > app_store_version:
logger.warning(
"Ignore app_store version %s since current version is newer %s",
latest_descriptor.version,
current_desc.get_version(),
)
return False

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think both are equivalent. I wrote it this way because that would be easy to add logs later if needed.
Also I find it easier to read but it's a personal preference.

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.

3 participants