-
Notifications
You must be signed in to change notification settings - Fork 26
feat: inform user about cli updates #91
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
Open
Guust-Franssens
wants to merge
14
commits into
microsoft:main
Choose a base branch
from
Guust-Franssens:feat/inform-about-user-about-cli-updates
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
929502f
feat: inform users whenever a new update is available
Guust-Franssens 28f9fe2
update docs
Guust-Franssens ee96752
rename change
Guust-Franssens 98da864
refactor always fetch pypi version on login
Guust-Franssens 620ff80
remove redundant logger message
Guust-Franssens 0dbd95c
refactor to use standalone tests
Guust-Franssens 87b8af1
use mock_fab_set_state_config instead of introducing own mock
Guust-Franssens c25c306
use mock_questionary_printinstead of own mock
Guust-Franssens ee5b9b4
test: ignore pypi.org requests in VCR cassettes
Guust-Franssens e3a65b1
chore: remove unnecessary comments
Guust-Franssens b56ef8e
more explicitly test disabled config
Guust-Franssens 2877e1e
Update src/fabric_cli/core/fab_constant.py
Guust-Franssens dd9daa8
accept suggestion for config name change
Guust-Franssens b384c46
add testing for logs
Guust-Franssens File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| kind: added | ||
| body: Display a notification to users on login when a new fab cli version is available | ||
| time: 2025-12-11T18:34:25.601088227+01:00 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| """ | ||
| Version update checking for Fabric CLI. | ||
|
|
||
| This module checks PyPI for newer versions of ms-fabric-cli and displays | ||
| a notification to the user if an update is available. | ||
| """ | ||
|
|
||
| from typing import Optional | ||
|
|
||
| import requests | ||
|
|
||
| from fabric_cli import __version__ | ||
| from fabric_cli.core import fab_constant, fab_logger, fab_state_config | ||
| from fabric_cli.utils import fab_ui | ||
|
|
||
|
|
||
| def _fetch_latest_version_from_pypi() -> Optional[str]: | ||
| """ | ||
| Fetch the latest version from PyPI JSON API. | ||
|
|
||
| Returns: | ||
| Latest version string if successful, None otherwise. | ||
| """ | ||
| try: | ||
| response = requests.get( | ||
| fab_constant.VERSION_CHECK_PYPI_URL, | ||
| timeout=fab_constant.VERSION_CHECK_TIMEOUT_SECONDS | ||
| ) | ||
| response.raise_for_status() | ||
| return response.json()["info"]["version"] | ||
| except (requests.RequestException, KeyError, ValueError, TypeError) as e: | ||
| # Silently fail - don't interrupt user experience for version checks | ||
| fab_logger.log_debug(f"Failed to fetch version from PyPI: {e}") | ||
| return None | ||
|
|
||
|
|
||
| def _is_pypi_version_newer(pypi_version: str) -> bool: | ||
| """ | ||
| Compare PyPI version with current version to determine if an update is available. | ||
|
|
||
| Args: | ||
| pypi_version: Version string from PyPI | ||
|
|
||
| Returns: | ||
| True if PyPI version is newer than current installed version | ||
| """ | ||
| try: | ||
| # Parse versions as tuples (e.g., "1.3.0" -> (1, 3, 0)) | ||
| current_parts = tuple(int(x) for x in __version__.split(".")) | ||
| pypi_parts = tuple(int(x) for x in pypi_version.split(".")) | ||
| return pypi_parts > current_parts | ||
| except (ValueError, AttributeError): | ||
| # Conservative: don't show notification version could not be parsed | ||
| return False | ||
|
|
||
|
|
||
| def check_and_notify_update() -> None: | ||
| """ | ||
| Check for CLI updates and display notification if a newer version is available. | ||
|
|
||
| This function: | ||
| - Respects user's check_updates config setting | ||
| - Checks PyPI on every login for the latest version | ||
| - Displays notification if an update is available | ||
| - Fails silently if PyPI is unreachable | ||
| """ | ||
| check_enabled = fab_state_config.get_config(fab_constant.FAB_CHECK_UPDATES) | ||
| if check_enabled == "false": | ||
| fab_logger.log_debug("Version check disabled by user configuration") | ||
| return | ||
|
|
||
| fab_logger.log_debug("Checking PyPI for latest version") | ||
| latest_version = _fetch_latest_version_from_pypi() | ||
|
|
||
| if latest_version and _is_pypi_version_newer(latest_version): | ||
| msg = ( | ||
| f"\n[notice] A new release of fab is available: {__version__} → {latest_version}\n" | ||
| "[notice] To update, run: pip install --upgrade ms-fabric-cli\n" | ||
| ) | ||
| fab_ui.print_grey(msg) | ||
| elif latest_version: | ||
| fab_logger.log_debug(f"Already on latest version: {__version__}") | ||
| else: | ||
| fab_logger.log_debug("Could not fetch latest version from PyPI") | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why did you used fab_logger.log_debug and not fab_ui.print_grey? using fab_logger.log_debug means user will see it only if fab_constant.FAB_DEBUG_ENABLED is set by the user to true.
Also, are those scenarios tested?
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'm using
fab_logger.log_debughere intentionally for the "already up to date" scenario. The rationale is:100% coverage on
fab_version_check.py: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.
agree for the use of fab_logger.log_debug
regarding tests - my Q was that if we verify the correct message is printed only when debug_enabled is true
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.
It was already tested before by this function, however I made it a bit more explicit by adding
mock_questionary_print.assert_not_called().fabric-cli/tests/test_utils/test_fab_version_check.py
Lines 164 to 175 in b56ef8e
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.
What’s missing is a check that the correct message is logged when debug_enabled=True in test_cli_version_check_same_version_success. You verified mock_questionary_print.assert_not_called(), but you should also assert that log.debug was called with the expected message.
use mock_fab_set_state_config(constant.FAB_DEBUG_ENABLED, True) to set debug_enabled=True then mock fab_logget.debug and verify the message
Same for: "test_cli_version_check_fetch_failure" and "test_cli_version_check_older_version_success"
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.
Apologies, I didn't know that you were referring to testing the logger outputs. I fixed it in b384c46