From 18f0ca68e3c71178a2266e8fb92905bb5a399633 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Fri, 3 May 2024 13:31:26 +0100 Subject: [PATCH 1/2] htmlchecker: Move _get_pattern() helper to Checker class This will be used to replace some very similar code in gitchecker. --- src/checkers/__init__.py | 23 +++++++++++++++++++++++ src/checkers/htmlchecker.py | 27 +++++---------------------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/checkers/__init__.py b/src/checkers/__init__.py index 5dccdc42..fb5cd687 100644 --- a/src/checkers/__init__.py +++ b/src/checkers/__init__.py @@ -180,6 +180,29 @@ def _substitute_template( except (KeyError, ValueError) as err: raise CheckerMetadataError("Error substituting template") from err + @classmethod + def _get_pattern( + cls, + checker_data: t.Dict, + pattern_name: str, + expected_groups: int = 1, + ) -> t.Optional[re.Pattern]: + try: + pattern_str = checker_data[pattern_name] + except KeyError: + return None + + try: + pattern = re.compile(pattern_str) + except re.error as err: + raise CheckerMetadataError(f"Invalid regex '{pattern_str}'") from err + if pattern.groups != expected_groups: + raise CheckerMetadataError( + f"Pattern '{pattern.pattern}' contains {pattern.groups} group(s) " + f"instead of {expected_groups}" + ) + return pattern + async def _complete_digests( self, url: t.Union[str, URL], digests: MultiDigest ) -> MultiDigest: diff --git a/src/checkers/htmlchecker.py b/src/checkers/htmlchecker.py index b953948a..1c2a140e 100644 --- a/src/checkers/htmlchecker.py +++ b/src/checkers/htmlchecker.py @@ -31,32 +31,13 @@ from ..lib import NETWORK_ERRORS, OPERATORS_SCHEMA from ..lib.externaldata import ExternalBase, ExternalData -from ..lib.errors import CheckerMetadataError, CheckerQueryError, CheckerFetchError +from ..lib.errors import CheckerQueryError, CheckerFetchError from . import Checker from ..lib.utils import filter_versioned_items, FallbackVersion log = logging.getLogger(__name__) -def _get_pattern( - checker_data: t.Dict, pattern_name: str, expected_groups: int = 1 -) -> t.Optional[re.Pattern]: - try: - pattern_str = checker_data[pattern_name] - except KeyError: - return None - try: - pattern = re.compile(pattern_str) - except re.error as err: - raise CheckerMetadataError(f"Invalid regex '{pattern_str}'") from err - if pattern.groups != expected_groups: - raise CheckerMetadataError( - f"Pattern '{pattern.pattern}' contains {pattern.groups} group(s) " - f"instead of {expected_groups}" - ) - return pattern - - def _semantic_version(version: str) -> semver.VersionInfo: try: return semver.VersionInfo.parse(version) @@ -157,8 +138,10 @@ async def check(self, external_data: ExternalBase): {f"parent_{k}": v for k, v in parent_json.items() if v is not None}, ) - combo_pattern = _get_pattern(external_data.checker_data, "pattern", 2) - version_pattern = _get_pattern(external_data.checker_data, "version-pattern", 1) + combo_pattern = self._get_pattern(external_data.checker_data, "pattern", 2) + version_pattern = self._get_pattern( + external_data.checker_data, "version-pattern", 1 + ) url_template = external_data.checker_data.get("url-template") sort_matches = external_data.checker_data.get("sort-matches", True) version_cls = _VERSION_SCHEMES[ From 1a0e0c76449a20344584644cca69c6c61d494af2 Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Fri, 3 May 2024 13:32:35 +0100 Subject: [PATCH 2/2] gitchecker: Use Checker._get_pattern helper Rather than asserting when given the wrong number of match groups, this helper raises a descriptive exception. This is both more helpful to the developer who has incorrectly specified the tag-pattern, but also seems to prevent an issue where an assertion failure causes the asyncio mainloop to hang. Fixes #418 Thanks to Tobias Micheler for finding the issue! --- src/checkers/gitchecker.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/checkers/gitchecker.py b/src/checkers/gitchecker.py index 35f04c9a..4a620515 100644 --- a/src/checkers/gitchecker.py +++ b/src/checkers/gitchecker.py @@ -92,13 +92,11 @@ async def check(self, external_data: ExternalBase): return await self._check_has_new(external_data) return await self._check_still_valid(external_data) - @staticmethod - async def _check_has_new(external_data: ExternalGitRepo): - tag_pattern = external_data.checker_data.get( - "tag-pattern", r"^(?:[vV])?((?:\d+\.)+\d+)$" - ) - tag_re = re.compile(tag_pattern) - assert tag_re.groups == 1 + @classmethod + async def _check_has_new(cls, external_data: ExternalGitRepo): + tag_re = cls._get_pattern(external_data.checker_data, "tag-pattern", 1) + if tag_re is None: + tag_re = re.compile(r"^(?:[vV])?((?:\d+\.)+\d+)$") version_scheme = external_data.checker_data.get("version-scheme", "loose") tag_cls = TAG_VERSION_SCHEMES[version_scheme] @@ -142,7 +140,7 @@ async def _check_has_new(external_data: ExternalGitRepo): except IndexError as err: raise CheckerQueryError( f"{external_data.current_version.url} has no tags matching " - f"'{tag_pattern}'" + f"'{tag_re.pattern}'" ) from err new_version = ExternalGitRef(