From c66ca8d726c039441b9628fd3bf5cdff7a8052f9 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 4 Aug 2026 01:26:52 +0500 Subject: [PATCH 1/3] fix: replace print() with logger.warning() in presets catalog warning Print to stderr is inappropriate for library code. Replaced with logger.warning() for proper log management. Removed unused sys import. --- src/specify_cli/presets/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets/__init__.py b/src/specify_cli/presets/__init__.py index cc5308f3fc..5867541bbb 100644 --- a/src/specify_cli/presets/__init__.py +++ b/src/specify_cli/presets/__init__.py @@ -10,6 +10,7 @@ import copy import json import hashlib +import logging import os import tempfile import shutil @@ -54,6 +55,8 @@ verify_archive_sha256, ) +logger = logging.getLogger(__name__) + _CONSTITUTION_PROVENANCE_FILE = ".constitution-template.json" @@ -4256,18 +4259,15 @@ def get_active_catalogs(self) -> List[PresetCatalogEntry]: Raises: PresetValidationError: If a catalog URL is invalid """ - import sys - # 1. SPECKIT_PRESET_CATALOG_URL env var replaces all defaults if env_value := os.environ.get("SPECKIT_PRESET_CATALOG_URL"): catalog_url = env_value.strip() self._validate_catalog_url(catalog_url) if catalog_url != self.DEFAULT_CATALOG_URL: if not getattr(self, "_non_default_catalog_warning_shown", False): - print( - "Warning: Using non-default preset catalog. " + logger.warning( + "Using non-default preset catalog. " "Only use catalogs from sources you trust.", - file=sys.stderr, ) self._non_default_catalog_warning_shown = True return [PresetCatalogEntry(url=catalog_url, name="custom", priority=1, install_allowed=True, description="Custom catalog via SPECKIT_PRESET_CATALOG_URL")] From 4960e4ee9e1ad48d924cd29cf20e8bd70595d5da Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Fri, 2 Oct 2026 23:10:29 +0500 Subject: [PATCH 2/3] fix(presets): retarget the logger warning after the preset package split presets/__init__.py is now a re-export module, so the branch's conflict resolved to main's version. The non-default catalog warning moved to presets/_catalog.py, where it was still written to stderr with print(); it now logs at WARNING through the module's own logger, and the local sys import it needed is gone. Merging main also dropped the logger definition this branch had added to __init__.py, which nothing imports. Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous) --- src/specify_cli/presets/_catalog.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets/_catalog.py b/src/specify_cli/presets/_catalog.py index a4768cc22d..5d359ab685 100644 --- a/src/specify_cli/presets/_catalog.py +++ b/src/specify_cli/presets/_catalog.py @@ -2,6 +2,7 @@ import hashlib import json +import logging import os import tempfile from dataclasses import dataclass @@ -20,6 +21,8 @@ ) from ._manifest import PresetError, PresetValidationError +logger = logging.getLogger(__name__) + @dataclass class PresetCatalogEntry: @@ -302,18 +305,15 @@ def get_active_catalogs(self) -> List[PresetCatalogEntry]: Raises: PresetValidationError: If a catalog URL is invalid """ - import sys - # 1. SPECKIT_PRESET_CATALOG_URL env var replaces all defaults if env_value := os.environ.get("SPECKIT_PRESET_CATALOG_URL"): catalog_url = env_value.strip() self._validate_catalog_url(catalog_url) if catalog_url != self.DEFAULT_CATALOG_URL: if not getattr(self, "_non_default_catalog_warning_shown", False): - print( - "Warning: Using non-default preset catalog. " - "Only use catalogs from sources you trust.", - file=sys.stderr, + logger.warning( + "Using non-default preset catalog. " + "Only use catalogs from sources you trust." ) self._non_default_catalog_warning_shown = True return [PresetCatalogEntry(url=catalog_url, name="custom", priority=1, install_allowed=True, description="Custom catalog via SPECKIT_PRESET_CATALOG_URL")] From 31883c33ebe27aa8d4441ff92d78bf95c8baaab4 Mon Sep 17 00:00:00 2001 From: Quratulain-bilal Date: Tue, 6 Oct 2026 16:26:41 +0500 Subject: [PATCH 3/3] test(presets): cover the catalog warning channel The logger conversion changed where the non-default catalog warning goes, and nothing asserted it. Two tests pin the new behaviour: the warning is captured at WARNING on specify_cli.presets._catalog while stderr stays empty, it is emitted once per catalog instance, and the default path logs nothing. Verified against the former print(..., file=sys.stderr) implementation: the channel test fails there - caplog is empty and the message lands on captured stderr - and passes after the fix. Assisted-by: opencode (model: mimo-v2.6-flash-free, autonomous) --- tests/specify_cli/presets/test_catalog.py | 37 +++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/specify_cli/presets/test_catalog.py b/tests/specify_cli/presets/test_catalog.py index 2ffbddfed9..343d4a5901 100644 --- a/tests/specify_cli/presets/test_catalog.py +++ b/tests/specify_cli/presets/test_catalog.py @@ -2,6 +2,7 @@ import io import json +import logging import tarfile import zipfile from contextlib import contextmanager @@ -251,6 +252,42 @@ def test_env_var_catalog_url(self, project_dir, monkeypatch): catalog = PresetCatalog(project_dir) assert catalog.get_catalog_url() == "https://custom.example.com/catalog.json" + def test_non_default_catalog_url_warns_through_logger_once( + self, project_dir, monkeypatch, caplog, capsys + ): + """The override warning reaches the logging framework, not stderr. + + Regresses the print(..., file=sys.stderr) implementation: that wrote + the message straight to stderr and logging captured nothing. + """ + monkeypatch.setenv( + "SPECKIT_PRESET_CATALOG_URL", "https://custom.example.com/catalog.json" + ) + catalog = PresetCatalog(project_dir) + caplog.set_level(logging.WARNING, logger="specify_cli.presets._catalog") + + assert catalog.get_catalog_url() == "https://custom.example.com/catalog.json" + + message = "Using non-default preset catalog" + assert message in caplog.text + assert message not in capsys.readouterr().err + + # Once per catalog instance: a second lookup must not repeat it. + catalog.get_catalog_url() + assert caplog.text.count(message) == 1 + + def test_default_catalog_url_logs_no_warning( + self, project_dir, monkeypatch, caplog + ): + """The default path must not emit the override warning.""" + monkeypatch.delenv("SPECKIT_PRESET_CATALOG_URL", raising=False) + catalog = PresetCatalog(project_dir) + caplog.set_level(logging.WARNING, logger="specify_cli.presets._catalog") + + catalog.get_catalog_url() + + assert "Using non-default preset catalog" not in caplog.text + # --- _make_request / GitHub auth --- def test_make_request_no_token_no_auth_header(self, project_dir, monkeypatch):