From c171a8ce1ec027dc4b8681cea93f41220997cd8f Mon Sep 17 00:00:00 2001 From: Ryan Clary <9618975+mrclary@users.noreply.github.com> Date: Mon, 14 Oct 2024 17:35:50 -0700 Subject: [PATCH] Apply suggestions from code review Co-authored-by: Carlos Cordoba --- .../tests/test_update_manager.py | 20 +++--- .../plugins/updatemanager/widgets/update.py | 2 +- spyder/plugins/updatemanager/workers.py | 70 ++++++++++++------- 3 files changed, 59 insertions(+), 33 deletions(-) diff --git a/spyder/plugins/updatemanager/tests/test_update_manager.py b/spyder/plugins/updatemanager/tests/test_update_manager.py index 30fac1e39fc..3a8da7c6c3f 100644 --- a/spyder/plugins/updatemanager/tests/test_update_manager.py +++ b/spyder/plugins/updatemanager/tests/test_update_manager.py @@ -13,7 +13,7 @@ from spyder.config.base import running_in_ci from spyder.plugins.updatemanager import workers from spyder.plugins.updatemanager.workers import ( - get_asset_info, WorkerUpdate, HTTP_ERROR_MSG + UpdateType, get_asset_info, WorkerUpdate, HTTP_ERROR_MSG ) from spyder.plugins.updatemanager.widgets import update from spyder.plugins.updatemanager.widgets.update import UpdateManagerWidget @@ -52,7 +52,7 @@ def test_updates(qtbot, mocker, caplog, version, channel): mocker.patch.object( UpdateManagerWidget, "start_update", new=lambda x: None ) - mocker.patch.object(workers, "CURR_VER", new=parse(version)) + mocker.patch.object(workers, "CURRENT_VERSION", new=parse(version)) mocker.patch.object( workers, "get_spyder_conda_channel", return_value=channel ) @@ -87,7 +87,7 @@ def test_updates(qtbot, mocker, caplog, version, channel): @pytest.mark.parametrize("stable_only", [True, False]) def test_update_non_stable(qtbot, mocker, version, release, stable_only): """Test we offer unstable updates.""" - mocker.patch.object(workers, "CURR_VER", new=parse(version)) + mocker.patch.object(workers, "CURRENT_VERSION", new=parse(version)) release = parse(release) worker = WorkerUpdate(stable_only) @@ -102,7 +102,7 @@ def test_update_non_stable(qtbot, mocker, version, release, stable_only): @pytest.mark.parametrize("version", ["4.0.0", "6.0.0"]) def test_update_no_asset(qtbot, mocker, version): """Test update availability when asset is not available""" - mocker.patch.object(workers, "CURR_VER", new=parse(version)) + mocker.patch.object(workers, "CURRENT_VERSION", new=parse(version)) releases = [parse("6.0.1"), parse("6.100.0")] worker = WorkerUpdate(True) @@ -117,11 +117,15 @@ def test_update_no_asset(qtbot, mocker, version): @pytest.mark.parametrize( "release,update_type", - [("6.0.1", "micro"), ("6.1.0", "minor"), ("7.0.0", "major")] + [ + ("6.0.1", UpdateType.Micro), + ("6.1.0", UpdateType.Minor), + ("7.0.0", UpdateType.Major) + ] ) @pytest.mark.parametrize("app", [True, False]) def test_get_asset_info(qtbot, mocker, release, update_type, app): - mocker.patch.object(workers, "CURR_VER", new=parse("6.0.0")) + mocker.patch.object(workers, "CURRENT_VERSION", new=parse("6.0.0")) mocker.patch.object(workers, "is_conda_based_app", return_value=app) info = get_asset_info(release) @@ -129,10 +133,10 @@ def test_get_asset_info(qtbot, mocker, release, update_type, app): if update_type == "major" or not app: assert info['url'].endswith(('.exe', '.pkg', '.sh')) - assert info['name'].endswith(('.exe', '.pkg', '.sh')) + assert info['filename'].endswith(('.exe', '.pkg', '.sh')) else: assert info['url'].endswith(".zip") - assert info['name'].endswith(".zip") + assert info['filename'].endswith(".zip") # ---- Test WorkerDownloadInstaller diff --git a/spyder/plugins/updatemanager/widgets/update.py b/spyder/plugins/updatemanager/widgets/update.py index 9cba9c085e5..afc20fbf6f8 100644 --- a/spyder/plugins/updatemanager/widgets/update.py +++ b/spyder/plugins/updatemanager/widgets/update.py @@ -254,7 +254,7 @@ def _set_installer_path(self): self.update_type = asset_info['update_type'] dirname = osp.join(get_temp_dir(), 'updates', str(self.latest_release)) - self.installer_path = osp.join(dirname, asset_info['name']) + self.installer_path = osp.join(dirname, asset_info['filename']) self.installer_size_path = osp.join(dirname, "size") logger.info(f"Update type: {self.update_type}") diff --git a/spyder/plugins/updatemanager/workers.py b/spyder/plugins/updatemanager/workers.py index bbd06f4341c..c2dfd90ca87 100644 --- a/spyder/plugins/updatemanager/workers.py +++ b/spyder/plugins/updatemanager/workers.py @@ -5,6 +5,7 @@ # (see spyder/__init__.py for details) # Standard library imports +from __future__ import annotations # noqa; required for typing in Python 3.8 from datetime import datetime as dt import logging import os @@ -14,10 +15,11 @@ import sys from time import sleep import traceback +from typing import TypedDict from zipfile import ZipFile # Third party imports -from packaging.version import parse +from packaging.version import parse, Version from qtpy.QtCore import QObject, Signal import requests from requests.exceptions import ConnectionError, HTTPError, SSLError @@ -31,7 +33,7 @@ # Logger setup logger = logging.getLogger(__name__) -CURR_VER = parse(__version__) +CURRENT_VERSION = parse(__version__) CONNECT_ERROR_MSG = _( 'Unable to connect to the Spyder update service.' @@ -67,7 +69,28 @@ def _rate_limits(page): logger.debug("\n\t".join(msg_items)) -def get_asset_info(release): +class UpdateType: + """Enum with the different update types.""" + + Major = "major" + Minor = "minor" + Micro = "micro" + + +class AssetInfo(TypedDict): + """Schema for asset information.""" + + # Filename with extension of the release asset to download. + filename: str + + # Type of update + update_type: UpdateType + + # Download URL for the asset. + url: str + + +def get_asset_info(release: str | Version) -> AssetInfo: """ Get the name, update type, and download URL for the asset of the given release. @@ -79,27 +102,22 @@ def get_asset_info(release): Returns ------- - asset_info: dict - name: str - Filename with extension of the release asset to download. - update_type: str - Type of update. One of {'major', 'minor', 'micro'}. - url: str - Download URL for the asset. + asset_info: AssetInfo + Information about the asset. """ if isinstance(release, str): release = parse(release) - if CURR_VER.major < release.major: - update_type = 'major' - elif CURR_VER.minor < release.minor: - update_type = 'minor' + if CURRENT_VERSION.major < release.major: + update_type = UpdateType.Major + elif CURRENT_VERSION.minor < release.minor: + update_type = UpdateType.Minor else: - update_type = 'micro' + update_type = UpdateType.Micro mach = platform.machine().lower().replace("amd64", "x86_64") - if update_type == 'major' or not is_conda_based_app(): + if update_type == UpdateType.Major or not is_conda_based_app(): if os.name == 'nt': plat, ext = 'Windows', 'exe' if sys.platform == 'darwin': @@ -115,7 +133,7 @@ def get_asset_info(release): f'v{release}/{name}' ) - return {'name': name, 'update_type': update_type, 'url': url} + return AssetInfo(filename=name, update_type=update_type, url=url) class UpdateDownloadCancelledException(Exception): @@ -174,15 +192,19 @@ def __init__(self, stable_only): self.error = None self.channel = None - def _check_update_available(self, releases, github=True): + def _check_update_available( + self, + releases: list[Version], + github: bool = True + ): """Checks if there is an update available from releases.""" if self.stable_only: # Only use stable releases releases = [r for r in releases if not r.is_prerelease] logger.debug(f"Available versions: {releases}") - latest_release = max(releases) if releases else CURR_VER - update_available = CURR_VER < latest_release + latest_release = max(releases) if releases else CURRENT_VERSION + update_available = CURRENT_VERSION < latest_release logger.debug(f"Latest release: {latest_release}") logger.debug(f"Update available: {update_available}") @@ -204,14 +226,14 @@ def _check_update_available(self, releases, github=True): # The asset is not available logger.debug( "Asset not available: " - f"{page.status_code} Client Error: {page.reason}" - f" for url: {page.url}" + f"{page.status_code} Client Error: {page.reason} " + f"for url: {page.url}" ) asset_available = False releases.remove(latest_release) - latest_release = max(releases) if releases else CURR_VER - update_available = CURR_VER < latest_release + latest_release = max(releases) if releases else CURRENT_VERSION + update_available = CURRENT_VERSION < latest_release logger.debug(f"Latest release: {latest_release}") logger.debug(f"Update available: {update_available}")