Skip to content

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.

4 participants