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

[Feature] Leo CLI new version update notification #28345

Open
wants to merge 4 commits into
base: mainnet
Choose a base branch
from

Conversation

ungaro
Copy link
Contributor

@ungaro ungaro commented Sep 11, 2024

Motivation

Implemented an update notification system to keep users informed about new versions of the Leo CLI tool.
This PR introduces the following enhancements to the update notification system:

  1. Conditional Update Checks: The update notification system performs checks based on a time interval (currently set to once per day) or when forced, reducing unnecessary network requests. It writes the last check timestamp and latest version to cache files. This check runs on any command with arguments.

  2. Integration with Help and Version Commands: Update notifications are also integrated into the help (--help) and version (--version) command outputs. These commands do not trigger an update check; instead, they read from the cached latest version file.

Closes #28335

@ungaro
Copy link
Contributor Author

ungaro commented Sep 11, 2024

needs review @d0cd

@d0cd
Copy link
Collaborator

d0cd commented Sep 14, 2024

Thanks for the contribution @ungaro!
Instead of caching the last time the file was checked, I would propose that updates are checked on every invocation and adding a CLI option to skip the update check.
Some benefits of this approach are:

  • it consistently reminds users of an update, while still allowing them to ignore it if they'd prefer.
  • avoids adding a cache directory (as a principle, we should mess with the folder structure as little as possible)
    Any opinions on the alternate design?

@ungaro
Copy link
Contributor Author

ungaro commented Sep 14, 2024

Well, I researched best practices before implementing this, and here are some examples of how tools handle checking for updates to themselves:

npm (Node.js package manager):

  • Checks for updates to npm itself weekly
  • Notifies users of available updates during npm CLI usage

Homebrew (macOS package manager):

  • Auto-updates itself by default when running brew install or brew upgrade
  • Can be configured to check less frequently

Cargo (Rust package manager):

  • Does not automatically check for updates to itself
  • Users typically update Cargo by updating their Rust installation

Git:

  • Does not automatically check for updates to itself
  • Users typically update Git through their system package manager

VS Code:

  • Checks for updates to itself on startup, but not more than once per day
  • Users can manually check through the UI

These examples show that while practices vary, many popular tools do implement some form of periodic self-update checking rather than checking on every invocation. This approach balances keeping users informed with avoiding excessive network requests and potential API rate limiting issues. (more about rate-limiting below)

Regarding checking every time, you'll still need some cache files. Once an update is found, it's unnecessary to check for new updates as leo update will still update to the latest version. Consider a scenario where there's an update from 2.1.0 to 2.2.0, and then a new update to 2.3.0, both happening on the same day. The only benefit of not using cache files would be to catch the 2.3.0 update notification immediately.

For --help and --version options, users often want to quickly check something (like leo version or a command reference) and may not prefer a network request delay.

Also, caching files doesn't prevent consistently reminding users of an update. It's consistently checking from the cache files and providing the notification in subsequent requests.

Moreover, frequent update checks might lead to rate limiting issues with GitHub's API. Many projects hosted on GitHub use their API for version checks, and GitHub does impose rate limits. For instance, unauthenticated requests are limited to 60 per hour per IP address. This could potentially cause problems for users in shared environments or those who frequently use the tool.

Caching also helps in offline scenarios, allowing the tool to provide the last known update information even when a network connection isn't available. if you're offline, you'll see some real lag as the connection will have to fail.

While the option to skip the update check is a good idea, having a reasonable default interval (like once a day) with the option to force a check seems to balance user awareness with system performance and API usage considerations. @d0cd

@d0cd
Copy link
Collaborator

d0cd commented Oct 7, 2024

Thank you for doing the research @ungaro! I agree with your analysis and like the overall approach. Left some comments on mainly focused on making the implementation more lightweight and canonical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants