From e64e8c7d53b1ed57db04b5e0d55a3debd7aa2e99 Mon Sep 17 00:00:00 2001 From: Tobias Gruetzmacher Date: Sun, 29 Oct 2023 19:05:20 +0100 Subject: [PATCH] Simplify update checker Motivated by the distutils removal, this replaces version parsing by a "good enough" version comparison algorithm... --- dosagelib/cmd.py | 19 +++++++---------- dosagelib/updater.py | 49 ++++++++++++++++++++++++-------------------- pyproject.toml | 4 ++-- tests/test_dosage.py | 10 ++++++++- 4 files changed, 45 insertions(+), 37 deletions(-) diff --git a/dosagelib/cmd.py b/dosagelib/cmd.py index b6c766b7d..134d3749f 100644 --- a/dosagelib/cmd.py +++ b/dosagelib/cmd.py @@ -1,7 +1,7 @@ # SPDX-License-Identifier: MIT -# Copyright (C) 2004-2008 Tristan Seligmann and Jonathan Jacobs -# Copyright (C) 2012-2014 Bastian Kleineidam -# Copyright (C) 2015-2022 Tobias Gruetzmacher +# SPDX-FileCopyrightText: © 2004 Tristan Seligmann and Jonathan Jacobs +# SPDX-FileCopyrightText: © 2012 Bastian Kleineidam +# SPDX-FileCopyrightText: © 2015 Tobias Gruetzmacher import argparse import os import platform @@ -124,8 +124,8 @@ def display_version(verbose): if verbose: # search for updates from .updater import check_update - result, value = check_update() - if result: + try: + value = check_update() if value: version, url = value if url is None: @@ -139,13 +139,8 @@ def display_version(verbose): attrs = {'version': version, 'app': AppName, 'url': url, 'currentversion': __version__} print(text % attrs) - else: - if value is None: - value = 'invalid update file syntax' - text = ('An error occured while checking for an ' - 'update of %(app)s: %(error)s.') - attrs = {'error': value, 'app': AppName} - print(text % attrs) + except (IOError, KeyError) as err: + print(f'An error occured while checking for an update of {AppName}: {err!r}') return 0 diff --git a/dosagelib/updater.py b/dosagelib/updater.py index 99218489d..3ef4ac512 100644 --- a/dosagelib/updater.py +++ b/dosagelib/updater.py @@ -1,10 +1,10 @@ # SPDX-License-Identifier: MIT -# Copyright (C) 2004-2008 Tristan Seligmann and Jonathan Jacobs -# Copyright (C) 2012-2014 Bastian Kleineidam -# Copyright (C) 2015-2020 Tobias Gruetzmacher +# SPDX-FileCopyrightText: © 2004 Tristan Seligmann and Jonathan Jacobs +# SPDX-FileCopyrightText: © 2012 Bastian Kleineidam +# SPDX-FileCopyrightText: © 2015 Tobias Gruetzmacher import os - -from distutils.version import LooseVersion +import re +from typing import Any import dosagelib from . import http @@ -18,35 +18,34 @@ EXTPRIO = { } -def check_update(): +def check_update() -> None | tuple[str, None | str]: """Return the following values: - (False, errmsg) - online version could not be determined - (True, None) - user has newest version - (True, (version, url string)) - update available - (True, (version, None)) - current version is newer than online version + throws exception - online version could not be determined + None - user has newest version + (version, url string) - update available + (version, None) - current version is newer than online version """ version, value = get_online_version() - if version is None: - # value is an error message - return False, value if version == dosagelib.__version__: # user has newest version - return True, None + return None if is_newer_version(version): # value is an URL linking to the update package - return True, (version, value) + return (version, value) # user is running a local or development version - return True, (version, None) + return (version, None) -def asset_key(asset): +def asset_key(asset: dict[str, Any]) -> int: return EXTPRIO.get(os.path.splitext(asset['browser_download_url'])[1], 99) -def get_online_version(): +def get_online_version() -> tuple[str, None | str]: """Download update info and parse it.""" - page = http.default_session.get(UPDATE_URL).json() - version = page.get('tag_name', None) + response = http.default_session.get(UPDATE_URL) + response.raise_for_status() + page = response.json() + version = page['tag_name'] url = None try: @@ -58,6 +57,12 @@ def get_online_version(): return version, url -def is_newer_version(version): +def version_nums(ver: str) -> tuple[int, ...]: + """Extract all numeric "sub-parts" of a version string. Not very exact, but + works for our use case.""" + return tuple(int(s) for s in re.split(r'\D+', ver + '0')) + + +def is_newer_version(version) -> bool: """Check if given version is newer than current version.""" - return LooseVersion(version) > LooseVersion(dosagelib.__version__) + return version_nums(version) > version_nums(dosagelib.__version__) diff --git a/pyproject.toml b/pyproject.toml index 923f77874..af7c92ea0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -52,16 +52,16 @@ dev = [ "pytest-cov", "pytest-xdist", "responses", - "setup-cfg-fmt", ] lint = [ - "flake8<6", + "flake8~=6.0", "flake8-2020", "flake8-breakpoint", "flake8-bugbear", "flake8-coding", "flake8-commas", "flake8-comprehensions", + "flake8-deprecated", "flake8-eradicate", "flake8-fixme", "flake8-functions", diff --git a/tests/test_dosage.py b/tests/test_dosage.py index 9b810a79e..b0c4d4179 100644 --- a/tests/test_dosage.py +++ b/tests/test_dosage.py @@ -82,7 +82,15 @@ class TestDosage: json={}) cmd_ok('--version', '-v') out = capfd.readouterr().out - assert 'invalid update file' in out + assert 'KeyError' in out + + @responses.activate + def test_update_rate_limit(self, capfd): + responses.add(responses.GET, re.compile(r'https://api\.github\.com/'), + status=403) + cmd_ok('--version', '-v') + out = capfd.readouterr().out + assert 'HTTPError' in out def test_display_help(self): for option in ("-h", "--help"):