From 0c62197fe8e3fbd408603405f37352a5b512a415 Mon Sep 17 00:00:00 2001 From: Claudio Cannizzaro Date: Thu, 29 Oct 2026 13:18:26 +0100 Subject: [PATCH 1/3] fix: fail with polars' rtcompat hint instead of SIGILL on CPUs without AVX2 polars only warns about missing CPU features and then the process dies with 'Illegal instruction'. Turn that one warning into an error when bfabric is imported, so the user sees the 'install polars[rtcompat]' hint. --- bfabric/docs/changelog.md | 1 + bfabric/src/bfabric/__init__.py | 4 ++++ tests/bfabric/test_cpu_check.py | 11 +++++++++++ 3 files changed, 16 insertions(+) create mode 100644 tests/bfabric/test_cpu_check.py diff --git a/bfabric/docs/changelog.md b/bfabric/docs/changelog.md index 924187497..6f95d1979 100644 --- a/bfabric/docs/changelog.md +++ b/bfabric/docs/changelog.md @@ -11,6 +11,7 @@ Minor breaking changes are still possible in `1.X.Y` but we try to announce them ### Fixed +- On a CPU without the features polars needs (e.g. AVX2), importing polars through `bfabric` raises polars' `RuntimeWarning` telling you to install `polars[rtcompat]`, instead of the process dying with "Illegal instruction". - OAuth requests to an instance recorded with an `http://` base URL are sent over `https://` instead of failing on the server's redirect (loopback hosts excepted). `bfabric.oauth.https_base_url` exposes the upgrade. ### Added diff --git a/bfabric/src/bfabric/__init__.py b/bfabric/src/bfabric/__init__.py index 6016a589a..377c0ed5d 100644 --- a/bfabric/src/bfabric/__init__.py +++ b/bfabric/src/bfabric/__init__.py @@ -1,10 +1,14 @@ import importlib.metadata +import warnings from bfabric.bfabric import Bfabric from bfabric.config.base_url import BaseUrl from bfabric.config.bfabric_auth import BfabricAuth from bfabric.config.bfabric_client_config import BfabricAPIEngineType, BfabricClientConfig +# polars only warns about missing CPU features and then dies with SIGILL; raise its install hint instead +warnings.filterwarnings("error", message="Missing required CPU features", category=RuntimeWarning) + __all__ = [ "BaseUrl", "Bfabric", diff --git a/tests/bfabric/test_cpu_check.py b/tests/bfabric/test_cpu_check.py new file mode 100644 index 000000000..ea8554e3a --- /dev/null +++ b/tests/bfabric/test_cpu_check.py @@ -0,0 +1,11 @@ +import subprocess +import sys + + +def test_polars_missing_cpu_features_warning_raises(): + # pytest replaces warning filters per test, so check the real import in a fresh interpreter; + # the message mimics polars' _cpu_check, emitted right before the process dies with SIGILL + code = "import bfabric, warnings; warnings.warn('Missing required CPU features. Install polars[rtcompat]', RuntimeWarning)" + result = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True) + assert result.returncode == 1 + assert "RuntimeWarning: Missing required CPU features" in result.stderr From 2da61c6eabc211d3636499958aa272d0b048cbd0 Mon Sep 17 00:00:00 2001 From: Claudio Cannizzaro Date: Thu, 29 Oct 2026 13:18:26 +0100 Subject: [PATCH 2/3] fix: report user errors from bfabric-cli in one line instead of a traceback Wrap suds faults, unknown query fields and lost connections in BfabricRequestError / BfabricUnavailableError, report ValueError, OSError, YAML and HTTP errors from use_client as a single Error: line, and add the report_user_errors context manager for the same handling elsewhere. An unknown config environment raises a ValueError listing the available ones. --- bfabric/docs/changelog.md | 5 ++- bfabric/src/bfabric/config/config_data.py | 2 +- bfabric/src/bfabric/config/config_file.py | 11 ++++- bfabric/src/bfabric/engine/engine_suds.py | 36 +++++++++++++-- bfabric/src/bfabric/utils/cli_integration.py | 23 +++++++--- bfabric_scripts/docs/changelog.md | 1 + .../src/bfabric_scripts/cli/__main__.py | 5 ++- .../src/bfabric_scripts/cli/api/parser.py | 8 +++- tests/bfabric/config/test_config_data.py | 6 +++ tests/bfabric/config/test_config_file.py | 6 +++ tests/bfabric/engine/test_engine_suds.py | 45 ++++++++++++++++++- tests/bfabric/utils/test_cli_integration.py | 39 ++++++++++++++++ .../bfabric_cli/test_cli_dataset_download.py | 10 +++-- tests/bfabric_cli/test_cli_main.py | 15 +++++++ tests/bfabric_scripts/test_api_parser.py | 12 +++++ 15 files changed, 199 insertions(+), 25 deletions(-) create mode 100644 tests/bfabric_cli/test_cli_main.py diff --git a/bfabric/docs/changelog.md b/bfabric/docs/changelog.md index 6f95d1979..b44c9df65 100644 --- a/bfabric/docs/changelog.md +++ b/bfabric/docs/changelog.md @@ -11,8 +11,11 @@ Minor breaking changes are still possible in `1.X.Y` but we try to announce them ### Fixed -- On a CPU without the features polars needs (e.g. AVX2), importing polars through `bfabric` raises polars' `RuntimeWarning` telling you to install `polars[rtcompat]`, instead of the process dying with "Illegal instruction". +- An unknown query field (e.g. `status` on an endpoint without it), a SOAP server fault, or a connection lost mid-request now raise `BfabricRequestError` / `BfabricUnavailableError` instead of raw suds exceptions, so `bfabric-cli` reports them in one line instead of a traceback. +- `use_client` reports a `ValueError`, `OSError`, YAML error or HTTP error status from the command or from loading the config (e.g. no config file yet) as one `Error:` line instead of a traceback; the new `report_user_errors` context manager applies the same handling elsewhere. +- An unknown config environment (e.g. a typo in `--config-env`) raises a `ValueError` listing the available environments instead of a `KeyError`. - OAuth requests to an instance recorded with an `http://` base URL are sent over `https://` instead of failing on the server's redirect (loopback hosts excepted). `bfabric.oauth.https_base_url` exposes the upgrade. +- On a CPU without the features polars needs (e.g. AVX2), importing polars through `bfabric` raises polars' `RuntimeWarning` telling you to install `polars[rtcompat]`, instead of the process dying with "Illegal instruction". ### Added diff --git a/bfabric/src/bfabric/config/config_data.py b/bfabric/src/bfabric/config/config_data.py index 6ac011237..2a3649323 100644 --- a/bfabric/src/bfabric/config/config_data.py +++ b/bfabric/src/bfabric/config/config_data.py @@ -93,7 +93,7 @@ def _load_environment_config_data(config_path: Path | str, force_config_env: str logger.debug(f"Reading configuration from: {config_file_path} {config_env=}") config_file = ConfigFile.model_validate(yaml.safe_load(config_file_path.read_text())) resolved_env = config_file.get_selected_config_env(explicit_config_env=config_env) - env_config = config_file.environments[resolved_env] + env_config = config_file.get_selected_config(explicit_config_env=resolved_env) return ConfigData( client=env_config.config, auth=env_config.auth, diff --git a/bfabric/src/bfabric/config/config_file.py b/bfabric/src/bfabric/config/config_file.py index 6db196024..b14a51784 100644 --- a/bfabric/src/bfabric/config/config_file.py +++ b/bfabric/src/bfabric/config/config_file.py @@ -136,8 +136,15 @@ def get_selected_config_env(self, explicit_config_env: str | None) -> str: return env def get_selected_config(self, explicit_config_env: str | None = None) -> EnvironmentConfig: - """Return the selected environment; see :meth:`get_selected_config_env`.""" - return self.environments[self.get_selected_config_env(explicit_config_env=explicit_config_env)] + """Return the selected environment; see :meth:`get_selected_config_env`. + + :raises ValueError: if no environment of that name is in the config + """ + name = self.get_selected_config_env(explicit_config_env=explicit_config_env) + if name not in self.environments: + available = ", ".join(sorted(self.environments)) + raise ValueError(f"Environment {name!r} not found in the config; available: {available}") + return self.environments[name] def read_config_file( diff --git a/bfabric/src/bfabric/engine/engine_suds.py b/bfabric/src/bfabric/engine/engine_suds.py index 9d016f984..bdfafb036 100644 --- a/bfabric/src/bfabric/engine/engine_suds.py +++ b/bfabric/src/bfabric/engine/engine_suds.py @@ -1,11 +1,12 @@ from __future__ import annotations import copy +from contextlib import contextmanager from typing import TYPE_CHECKING, Any from urllib.error import URLError import suds.transport -from suds import MethodNotFound +from suds import MethodNotFound, TypeNotFound, WebFault from suds.client import Client from bfabric.engine.response_format_suds import suds_asdict_recursive @@ -15,6 +16,8 @@ from bfabric.results.result_container import ResultContainer if TYPE_CHECKING: + from collections.abc import Iterator + from suds.serviceproxy import ServiceProxy from bfabric.config import BaseUrl, BfabricAuth @@ -58,7 +61,8 @@ def read( "idonly": return_id_only, } service = self._get_suds_service(endpoint) - response = service.read(full_query) + with self._translate_call_errors(endpoint): + response = service.read(full_query) return self._convert_results(response=response, endpoint=endpoint) def save( @@ -78,9 +82,11 @@ def save( query = {"login": auth.login, "password": auth.password.get_secret_value(), endpoint: obj} service = self._get_suds_service(endpoint) try: - response = getattr(service, method)(query) + save_method = getattr(service, method) except MethodNotFound as e: raise BfabricRequestError(f"SUDS failed to find save method for the {endpoint} endpoint.") from e + with self._translate_call_errors(endpoint): + response = save_method(query) return self._convert_results(response=response, endpoint=endpoint) def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> ResultContainer: @@ -93,7 +99,8 @@ def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> Resul return ResponseDelete.from_empty_request() query = {"login": auth.login, "password": auth.password.get_secret_value(), "id": id} service = self._get_suds_service(endpoint) - response = service.delete(query) + with self._translate_call_errors(endpoint): + response = service.delete(query) return ResponseDelete.from_suds(suds_response=response, endpoint=endpoint) def _get_suds_service(self, endpoint: str) -> ServiceProxy: @@ -116,6 +123,27 @@ def _get_suds_service(self, endpoint: str) -> ServiceProxy: raise BfabricUnavailableError(self._base_url, error) from error return self._cl[endpoint].service + @contextmanager + def _translate_call_errors(self, endpoint: str) -> Iterator[None]: + """Translate the suds exceptions a service call raises into errors the callers handle. + + None of them is a ``RuntimeError``, so untranslated a typo in a query field (``TypeNotFound``) + escapes the CLI's error handling as a traceback. suds reports an HTTP error status (e.g. a proxy's + 502) as a bare ``Exception((status, reason))``, hence the catch-all. + """ + try: + yield + except TypeNotFound as error: + # suds keeps the name only in its message, "Type not found: ''" + name = str(error).removeprefix("Type not found: ") + raise BfabricRequestError(f"Unknown field {name} for the {endpoint!r} endpoint.") from error + except WebFault as error: + raise BfabricRequestError(f"B-Fabric server fault on the {endpoint!r} endpoint: {error}") from error + except OSError as error: + raise BfabricUnavailableError(self._base_url, error) from error + except Exception as error: + raise BfabricRequestError(f"Request to the {endpoint!r} endpoint failed: {error}") from error + def _convert_results(self, response: Any, endpoint: str) -> ResultContainer: try: n_available_pages = response["numberofpages"] diff --git a/bfabric/src/bfabric/utils/cli_integration.py b/bfabric/src/bfabric/utils/cli_integration.py index f18db3baf..69f635e9c 100644 --- a/bfabric/src/bfabric/utils/cli_integration.py +++ b/bfabric/src/bfabric/utils/cli_integration.py @@ -5,9 +5,12 @@ import os import sys import traceback +from contextlib import contextmanager from pathlib import Path from typing import TYPE_CHECKING, Annotated, NoReturn, TypeVar, cast +import httpx +import yaml from loguru import logger from rich.highlighter import RegexHighlighter from rich.theme import Theme @@ -15,7 +18,7 @@ from bfabric.config import DEFAULT_CONFIG_FILE if TYPE_CHECKING: - from collections.abc import Callable + from collections.abc import Callable, Iterator T = TypeVar("T") @@ -69,22 +72,28 @@ def wrapper(*args: object, **kwargs: object) -> T: if "client" in kwargs: client = cast("Bfabric", kwargs.pop("client")) else: - try: + with report_user_errors(): client = Bfabric.connect( config_file_path=config_file or DEFAULT_CONFIG_FILE, config_file_env=config_env or "default", ) - except (ValueError, RuntimeError) as e: - _report_and_exit(e) - try: + with report_user_errors(): return fn(*args, client=client, **kwargs) # type: ignore[arg-type] - except RuntimeError as e: - _report_and_exit(e) wrapper.__signature__ = new_sig # type: ignore[reportAttributeAccessIssue] return wrapper +@contextmanager +def report_user_errors() -> Iterator[None]: + """Report an error a user can cause (bad argument, missing file, unreadable config, entity not found, + HTTP error status) as one stderr line and exit 1.""" + try: + yield + except (RuntimeError, ValueError, OSError, yaml.YAMLError, httpx.HTTPStatusError) as e: + _report_and_exit(e) + + def _report_and_exit(error: Exception) -> NoReturn: """Reports ``error`` as a single stderr line and exits 1, keeping the traceback for DEBUG runs.""" # Not logger.opt(exception=...): the DEBUG sink runs with loguru's default diagnose=True, which diff --git a/bfabric_scripts/docs/changelog.md b/bfabric_scripts/docs/changelog.md index 65d49c092..062667c04 100644 --- a/bfabric_scripts/docs/changelog.md +++ b/bfabric_scripts/docs/changelog.md @@ -2,6 +2,7 @@ ### Fixed +- `bfabric-cli` reports user errors in one `Error:` line instead of a traceback: an unknown field (e.g. `api read ... --query status ...`), a missing file or config, a missing entity, a malformed config or JSON, an HTTP error from the OAuth endpoints, and an unknown method in `api inspect`. Set `BFABRICPY_LOG_LEVEL=DEBUG` to see the traceback. - `bfabric-cli login` and `auth device-code` no longer fail on an environment recorded with an `http://` base URL; they use `https://` and save the corrected URL. ### Changed diff --git a/bfabric_scripts/src/bfabric_scripts/cli/__main__.py b/bfabric_scripts/src/bfabric_scripts/cli/__main__.py index 42573f51f..17be4124b 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/__main__.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/__main__.py @@ -2,7 +2,7 @@ import cyclopts -from bfabric.utils.cli_integration import setup_script_logging +from bfabric.utils.cli_integration import report_user_errors, setup_script_logging from bfabric_scripts.cli.cli_api import cmd_api from bfabric_scripts.cli.cli_dataset import cmd_dataset from bfabric_scripts.cli.cli_executable import cmd_executable @@ -31,7 +31,8 @@ def main() -> None: """CLI entry point; sets logging up here because commands without ``@use_client`` would otherwise run at loguru's default DEBUG level.""" setup_script_logging() - app() + with report_user_errors(): + app() if __name__ == "__main__": diff --git a/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py b/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py index 3564f8e90..aed63cfa3 100644 --- a/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py +++ b/bfabric_scripts/src/bfabric_scripts/cli/api/parser.py @@ -9,6 +9,7 @@ from typing import Any from pydantic import BaseModel, Field +from suds import MethodNotFound # pyright: ignore[reportMissingTypeStubs] from suds.xsd.query import TypeQuery # pyright: ignore[reportMissingTypeStubs] from bfabric import Bfabric @@ -45,7 +46,7 @@ def parse_method_signature( """Parses a SOAP method signature into reusable Pydantic models. :raises RuntimeError: If the client is not configured to use the SUDS engine. - :raises AttributeError: If the endpoint or method does not exist. + :raises ValueError: If the method does not exist on the endpoint. """ # The WSDL introspection below is written against SUDS internals, so reject any other engine up # front instead of leaking an AttributeError from the private engine. @@ -57,7 +58,10 @@ def parse_method_signature( service = client._engine._get_suds_service(endpoint) # type: ignore[attr-defined] # pyright: ignore[reportPrivateUsage,reportAttributeAccessIssue,reportUnknownVariableType,reportUnknownMemberType] - method = getattr(service, method_name) # pyright: ignore[reportAny,reportUnknownArgumentType] + try: + method = getattr(service, method_name) # pyright: ignore[reportAny,reportUnknownArgumentType] + except MethodNotFound as error: + raise ValueError(f"Unknown method {method_name!r} for the {endpoint!r} endpoint.") from error binding = method.method.binding.input # pyright: ignore[reportAny] param_defs = binding.param_defs(method.method) # pyright: ignore[reportAny] diff --git a/tests/bfabric/config/test_config_data.py b/tests/bfabric/config/test_config_data.py index e7e73010f..ef9f2d121 100644 --- a/tests/bfabric/config/test_config_data.py +++ b/tests/bfabric/config/test_config_data.py @@ -105,6 +105,12 @@ def test_from_file_and_file_does_not_exist(self, mocker): with pytest.raises(OSError, match="No explicit config provided, and no config file found at"): load_config_data(config_file_path="/dev/null", config_file_env="TEST", include_auth=True) + def test_from_file_unknown_env(self, mocker, example_config_path): + mock_env = mocker.patch.dict(os.environ) + mock_env.pop("BFABRICPY_CONFIG_OVERRIDE", None) + with pytest.raises(ValueError, match="Environment 'NOSUCHENV' not found in the config"): + load_config_data(config_file_path=example_config_path, config_file_env="NOSUCHENV", include_auth=True) + def test_export_roundtrip(config_data): exported = export_config_data(config_data) diff --git a/tests/bfabric/config/test_config_file.py b/tests/bfabric/config/test_config_file.py index d4541cd99..f980ae837 100644 --- a/tests/bfabric/config/test_config_file.py +++ b/tests/bfabric/config/test_config_file.py @@ -155,6 +155,12 @@ def test_get_selected_config(config_with_auth, mocker): mock_get_config_env.assert_called_once_with(explicit_config_env=None) +def test_get_selected_config_unknown_env(config_with_auth): + """A typo in ``--config-env`` names the available environments instead of raising a bare KeyError.""" + with pytest.raises(ValueError, match=r"Environment 'PRODUCTOIN' not found in the config; available: .*PRODUCTION"): + config_with_auth.get_selected_config("PRODUCTOIN") + + def test_reject_env_name_default(mocker, data_no_auth): data_no_auth["default"] = {"base_url": "https://example.com/bfabric"} with pytest.raises(ValueError) as error: diff --git a/tests/bfabric/engine/test_engine_suds.py b/tests/bfabric/engine/test_engine_suds.py index 2159bf2f8..52be05b30 100644 --- a/tests/bfabric/engine/test_engine_suds.py +++ b/tests/bfabric/engine/test_engine_suds.py @@ -3,7 +3,7 @@ import pytest import suds.transport from pydantic import SecretStr -from suds import MethodNotFound +from suds import MethodNotFound, TypeNotFound, WebFault from suds.client import Client from bfabric.engine.engine_suds import EngineSUDS @@ -67,7 +67,8 @@ def test_save(engine_suds, mock_auth, mock_suds_service, mocker): def test_save_method_not_found(engine_suds, mock_auth, mock_suds_service, mocker): - mock_suds_service.save.side_effect = MethodNotFound("save") + # suds raises it on the attribute lookup, not on the call + type(mock_suds_service).save = mocker.PropertyMock(side_effect=MethodNotFound("save")) with pytest.raises( BfabricRequestError, @@ -171,6 +172,46 @@ def test_missing_endpoint_still_reports_the_endpoint(self, mocker): assert not isinstance(exc_info.value, BfabricUnavailableError) +class TestServiceCallErrors: + """Errors raised by suds during a service call must arrive as a RuntimeError, not a traceback.""" + + @pytest.fixture(params=["read", "save", "delete"]) + def call(self, request, engine_suds, mock_auth, mock_suds_service): + method = request.param + args = {"read": {}, "save": {}, "delete": 1}[method] + return getattr(mock_suds_service, method), lambda: getattr(engine_suds, method)("sample", args, mock_auth) + + def test_unknown_field(self, call): + service_method, run = call + service_method.side_effect = TypeNotFound("status") + + with pytest.raises(BfabricRequestError) as exc_info: + run() + assert str(exc_info.value) == "Unknown field 'status' for the 'sample' endpoint." + + def test_server_fault(self, call, mocker): + service_method, run = call + service_method.side_effect = WebFault(mocker.MagicMock(faultstring="boom"), None) + + with pytest.raises(BfabricRequestError, match="boom"): + run() + + def test_http_error_status(self, call): + """suds reports a non-SOAP HTTP error status from a call as a bare ``Exception((status, reason))``.""" + service_method, run = call + service_method.side_effect = Exception((502, "Bad Gateway")) + + with pytest.raises(BfabricRequestError, match="502"): + run() + + def test_connection_failure(self, call): + service_method, run = call + service_method.side_effect = URLError(ConnectionResetError(104, "Connection reset")) + + with pytest.raises(BfabricUnavailableError): + run() + + def test_convert_results_no_results(engine_suds, mocker): mock_response = mocker.MagicMock() mock_response.numberofpages = 0 diff --git a/tests/bfabric/utils/test_cli_integration.py b/tests/bfabric/utils/test_cli_integration.py index 915ef3e46..92f343555 100644 --- a/tests/bfabric/utils/test_cli_integration.py +++ b/tests/bfabric/utils/test_cli_integration.py @@ -9,7 +9,9 @@ import typing from pathlib import Path +import httpx import pytest +import yaml from bfabric.utils import cli_integration from bfabric.utils.cli_integration import setup_script_logging, use_client @@ -326,6 +328,43 @@ def my_function(*, client: Bfabric) -> None: assert exc_info.value.code == 1 assert capsys.readouterr().err == "Error: no config\n" + @pytest.mark.parametrize( + "error", + [ + ValueError("no such dataset"), + FileNotFoundError("missing.csv"), + httpx.HTTPStatusError( + "400 Bad Request", request=httpx.Request("POST", "https://x"), response=httpx.Response(400) + ), + ], + ) + def test_user_error_from_the_command_is_reported_as_one_line(self, mocker, capsys, error): + mocker.patch("bfabric.Bfabric.connect", return_value=mocker.Mock()) + + @use_client + def my_function(*, client: Bfabric) -> None: + raise error + + with pytest.raises(SystemExit) as exc_info: + my_function() + + assert exc_info.value.code == 1 + assert capsys.readouterr().err == f"Error: {error}\n" + + @pytest.mark.parametrize("error", [OSError("no config file found"), yaml.YAMLError("bad yaml")]) + def test_unreadable_config_is_reported_as_one_line(self, mocker, capsys, error): + mocker.patch("bfabric.Bfabric.connect", side_effect=error) + + @use_client + def my_function(*, client: Bfabric) -> None: + raise AssertionError("must not be reached") + + with pytest.raises(SystemExit) as exc_info: + my_function() + + assert exc_info.value.code == 1 + assert capsys.readouterr().err == f"Error: {error}\n" + def test_traceback_is_kept_at_debug_level(self, mocker, mock_logger): """The traceback is still available for development via ``BFABRICPY_LOG_LEVEL=DEBUG``.""" mocker.patch("bfabric.Bfabric.connect", return_value=mocker.Mock()) diff --git a/tests/bfabric_cli/test_cli_dataset_download.py b/tests/bfabric_cli/test_cli_dataset_download.py index 635745e7d..3cc946210 100644 --- a/tests/bfabric_cli/test_cli_dataset_download.py +++ b/tests/bfabric_cli/test_cli_dataset_download.py @@ -138,19 +138,21 @@ def test_download_auto_format_excel(self, mocker, mock_client, mock_dataset, tmp mock_dataset.to_polars.return_value.write_excel.assert_called_once() - def test_download_auto_format_unknown_extension(self, mocker, mock_client, mock_dataset): + def test_download_auto_format_unknown_extension(self, mocker, mock_client, mock_dataset, capsys): mocker.patch.object(Dataset, "find", return_value=mock_dataset) params = Params(dataset_id=123, file=Path("output.txt"), format=OutputFormat.AUTO) - with pytest.raises(ValueError, match="Cannot infer format from file extension"): + with pytest.raises(SystemExit): cmd_dataset_download(params, client=mock_client) + assert "Cannot infer format from file extension" in capsys.readouterr().err - def test_download_dataset_not_found(self, mocker, mock_client): + def test_download_dataset_not_found(self, mocker, mock_client, capsys): mocker.patch.object(Dataset, "find", return_value=None) params = Params(dataset_id=999, file=Path("output.csv"), format=OutputFormat.CSV) - with pytest.raises(ValueError, match="Dataset with id 999 not found"): + with pytest.raises(SystemExit): cmd_dataset_download(params, client=mock_client) + assert capsys.readouterr().err == "Error: Dataset with id 999 not found.\n" def test_download_creates_directory(self, mocker, mock_client, mock_dataset, tmp_path): mocker.patch.object(Dataset, "find", return_value=mock_dataset) diff --git a/tests/bfabric_cli/test_cli_main.py b/tests/bfabric_cli/test_cli_main.py new file mode 100644 index 000000000..6dd3f27d4 --- /dev/null +++ b/tests/bfabric_cli/test_cli_main.py @@ -0,0 +1,15 @@ +import pytest + +from bfabric_scripts.cli import __main__ as cli_main + + +def test_main_reports_a_command_error_in_one_line(mocker, capsys): + """Commands without ``@use_client`` (e.g. ``auth``) share the same one-line error boundary.""" + mocker.patch.object(cli_main, "setup_script_logging") + mocker.patch.object(cli_main, "app", side_effect=ValueError("base URL must end in /bfabric")) + + with pytest.raises(SystemExit) as exc_info: + cli_main.main() + + assert exc_info.value.code == 1 + assert capsys.readouterr().err == "Error: base URL must end in /bfabric\n" diff --git a/tests/bfabric_scripts/test_api_parser.py b/tests/bfabric_scripts/test_api_parser.py index 976d5282f..77ebe9586 100644 --- a/tests/bfabric_scripts/test_api_parser.py +++ b/tests/bfabric_scripts/test_api_parser.py @@ -4,6 +4,7 @@ import pytest from pytest_mock import MockerFixture +from suds import MethodNotFound from bfabric.config.bfabric_client_config import BfabricAPIEngineType from bfabric_scripts.cli.api.parser import ( @@ -295,6 +296,17 @@ def test_parse_method_signature_rejects_non_suds_engine(self, mocker: MockerFixt # Guard must fire before any private engine access. client_mock._engine._get_suds_service.assert_not_called() + def test_parse_method_signature_unknown_method(self, mocker: MockerFixture) -> None: + """suds raises ``MethodNotFound`` on the lookup; it must arrive as a one-line ``ValueError``.""" + service_mock = mocker.MagicMock() + type(service_mock).raed = mocker.PropertyMock(side_effect=MethodNotFound("raed")) + client_mock = mocker.MagicMock() + client_mock.config.engine = BfabricAPIEngineType.SUDS + client_mock._engine._get_suds_service.return_value = service_mock + + with pytest.raises(ValueError, match="'raed'.*'resource'"): + parse_method_signature(client_mock, "resource", "raed") + if __name__ == "__main__": pytest.main([__file__, "-v"]) From e22743b490d5ed00f04a8e99a5c8abf745f699ba Mon Sep 17 00:00:00 2001 From: Claudio Cannizzaro Date: Thu, 29 Oct 2026 13:23:20 +0100 Subject: [PATCH 3/3] ci: fix basedpyright and asgi_auth tests broken by newer pydantic and pytest A newer pydantic makes basedpyright reject TypeAdapter(Annotated alias); add targeted ignores. Wrapping the suds calls in _translate_call_errors moved the baseline-grandfathered reportAny lines, so ignore them in place (basedpyright pruned the now-stale baseline entries). pytest 9.2 removed FixtureManager._arg2fixturedefs, which pytest-bdd 9.0 still uses; cap pytest in the asgi_auth test extra. --- .basedpyright/baseline.bfabric.json | 40 --------------------- bfabric/src/bfabric/engine/engine_suds.py | 8 ++--- bfabric/src/bfabric/entities/application.py | 2 +- bfabric/src/bfabric/entities/core/uri.py | 4 ++- bfabric_asgi_auth/pyproject.toml | 2 +- 5 files changed, 9 insertions(+), 47 deletions(-) diff --git a/.basedpyright/baseline.bfabric.json b/.basedpyright/baseline.bfabric.json index 8314f0cb8..3620b9af0 100644 --- a/.basedpyright/baseline.bfabric.json +++ b/.basedpyright/baseline.bfabric.json @@ -235,46 +235,6 @@ "lineCount": 1 } }, - { - "code": "reportAny", - "range": { - "startColumn": 8, - "endColumn": 16, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 19, - "endColumn": 31, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 12, - "endColumn": 20, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 8, - "endColumn": 16, - "lineCount": 1 - } - }, - { - "code": "reportAny", - "range": { - "startColumn": 19, - "endColumn": 33, - "lineCount": 1 - } - }, { "code": "reportUnknownMemberType", "range": { diff --git a/bfabric/src/bfabric/engine/engine_suds.py b/bfabric/src/bfabric/engine/engine_suds.py index bdfafb036..5adf091f7 100644 --- a/bfabric/src/bfabric/engine/engine_suds.py +++ b/bfabric/src/bfabric/engine/engine_suds.py @@ -62,7 +62,7 @@ def read( } service = self._get_suds_service(endpoint) with self._translate_call_errors(endpoint): - response = service.read(full_query) + response = service.read(full_query) # pyright: ignore[reportAny] return self._convert_results(response=response, endpoint=endpoint) def save( @@ -82,11 +82,11 @@ def save( query = {"login": auth.login, "password": auth.password.get_secret_value(), endpoint: obj} service = self._get_suds_service(endpoint) try: - save_method = getattr(service, method) + save_method = getattr(service, method) # pyright: ignore[reportAny] except MethodNotFound as e: raise BfabricRequestError(f"SUDS failed to find save method for the {endpoint} endpoint.") from e with self._translate_call_errors(endpoint): - response = save_method(query) + response = save_method(query) # pyright: ignore[reportAny] return self._convert_results(response=response, endpoint=endpoint) def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> ResultContainer: @@ -100,7 +100,7 @@ def delete(self, endpoint: str, id: int | list[int], auth: BfabricAuth) -> Resul query = {"login": auth.login, "password": auth.password.get_secret_value(), "id": id} service = self._get_suds_service(endpoint) with self._translate_call_errors(endpoint): - response = service.delete(query) + response = service.delete(query) # pyright: ignore[reportAny] return ResponseDelete.from_suds(suds_response=response, endpoint=endpoint) def _get_suds_service(self, endpoint: str) -> ServiceProxy: diff --git a/bfabric/src/bfabric/entities/application.py b/bfabric/src/bfabric/entities/application.py index 52853bda6..a8d55d93e 100644 --- a/bfabric/src/bfabric/entities/application.py +++ b/bfabric/src/bfabric/entities/application.py @@ -31,7 +31,7 @@ def technology_folder_name(self) -> PathSafeStr: if not _is_technology_list(technology_raw): raise ValueError("Technology must be a list of strings") technology = sorted(technology_raw)[0] - return TypeAdapter(PathSafeStr).validate_python(technology) + return TypeAdapter(PathSafeStr).validate_python(technology) # pyright: ignore[reportArgumentType] def _is_technology_list(technology: ApiResponseDataType) -> TypeGuard[list[str]]: diff --git a/bfabric/src/bfabric/entities/core/uri.py b/bfabric/src/bfabric/entities/core/uri.py index a35eceabf..a7dadc2de 100644 --- a/bfabric/src/bfabric/entities/core/uri.py +++ b/bfabric/src/bfabric/entities/core/uri.py @@ -68,7 +68,9 @@ def invalid(reason: str) -> ValueError: ValidatedEntityUri = Annotated[str, AfterValidator(_validate_entity_uri)] -entity_uri_adapter: TypeAdapter[str] = TypeAdapter(ValidatedEntityUri) +entity_uri_adapter: TypeAdapter[str] = TypeAdapter( # pyright: ignore[reportUnknownVariableType] + ValidatedEntityUri # pyright: ignore[reportArgumentType] +) class EntityUri(str): diff --git a/bfabric_asgi_auth/pyproject.toml b/bfabric_asgi_auth/pyproject.toml index 0809673ae..c9c21091c 100644 --- a/bfabric_asgi_auth/pyproject.toml +++ b/bfabric_asgi_auth/pyproject.toml @@ -27,7 +27,7 @@ demo = [ ] test = [ "bfabric-asgi-auth[demo]", - "pytest>=8.0.0", + "pytest>=8.0.0,<9.2", # shortcut: pytest 9.2 removed an internal pytest-bdd 9.0 uses; lift once pytest-bdd supports it "pytest-mock>=3.15.1", "pytest-asyncio>=1.2.0", "pytest-bdd>=8.1.0",