-
Notifications
You must be signed in to change notification settings - Fork 9
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
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 |
There was a problem hiding this comment.
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?
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 |
There was a problem hiding this comment.
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.
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.