From d6b13e569ae65eba38545e294cee0a6d1c740a01 Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Fri, 13 Mar 2026 10:46:14 +0100 Subject: [PATCH 1/6] feat(utils): make BaseService async Co-Authored-By: Claude Sonnet 4.6 --- src/aignostics/CLAUDE.md | 4 +- src/aignostics/application/_cli.py | 3 +- .../_gui/_page_application_describe.py | 3 +- src/aignostics/application/_service.py | 4 +- src/aignostics/bucket/_service.py | 4 +- src/aignostics/dataset/_service.py | 4 +- src/aignostics/gui/_frame.py | 2 +- src/aignostics/notebook/_service.py | 4 +- src/aignostics/platform/_service.py | 4 +- src/aignostics/qupath/_service.py | 4 +- src/aignostics/system/_cli.py | 5 +- src/aignostics/system/_gui.py | 8 +- src/aignostics/system/_service.py | 12 +- src/aignostics/utils/CLAUDE.md | 31 ++++- src/aignostics/utils/_service.py | 55 +++++++- src/aignostics/wsi/_service.py | 4 +- tests/aignostics/platform/service_test.py | 4 +- tests/aignostics/utils/service_test.py | 123 ++++++++++++++++++ 18 files changed, 237 insertions(+), 41 deletions(-) create mode 100644 tests/aignostics/utils/service_test.py diff --git a/src/aignostics/CLAUDE.md b/src/aignostics/CLAUDE.md index 6f1cb9c1b..a50fa7372 100644 --- a/src/aignostics/CLAUDE.md +++ b/src/aignostics/CLAUDE.md @@ -294,10 +294,10 @@ For detailed information about each module, see: from aignostics.utils import BaseService class Service(BaseService): - def health(self) -> Health: + async def health(self) -> Health: return Health(status=Health.Code.UP) - def info(self, mask_secrets=True) -> dict: + async def info(self, mask_secrets=True) -> dict: return {"version": "1.0.0"} ``` diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 29a069e9e..475a6da8e 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -1,5 +1,6 @@ """CLI of application module.""" +import asyncio import json import sys import time @@ -129,7 +130,7 @@ def _abort_if_system_unhealthy() -> None: - health = SystemService.health_static() + health = asyncio.run(SystemService.health_static()) if not health: logger.error(f"Platform is not healthy: {health.reason}. Aborting.") console.print(f"[error]Error:[/error] Platform is not healthy: {health.reason}. Aborting.") diff --git a/src/aignostics/application/_gui/_page_application_describe.py b/src/aignostics/application/_gui/_page_application_describe.py index 4d4a19bf4..028c0856d 100644 --- a/src/aignostics/application/_gui/_page_application_describe.py +++ b/src/aignostics/application/_gui/_page_application_describe.py @@ -107,6 +107,7 @@ async def _page_application_describe(application_id: str) -> None: # noqa: C901 type="positive", ) spinner.set_visibility(False) + system_healthy = bool(await SystemService.health_static()) if application is None: await _frame( @@ -301,8 +302,6 @@ def _add_application_version_selection_section() -> None: with ui.column(), ui.button(icon="info", on_click=info_dialog.open): ui.tooltip("Show changes and input/ouput schema of this application version.") with ui.stepper_navigation(): - # Check system health and determine if force option should be available - system_healthy = bool(SystemService.health_static()) unhealthy_tooltip = None with ui.button( "Next", diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index f28a9b284..e4f02dc00 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -73,7 +73,7 @@ def __init__(self) -> None: """Initialize service.""" super().__init__(Settings) # automatically loads and validates the settings - def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 """Determine info of this service. Args: @@ -84,7 +84,7 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PL """ return {} - def health(self) -> Health: # noqa: PLR6301 + async def health(self) -> Health: # noqa: PLR6301 """Determine health of this service. Returns: diff --git a/src/aignostics/bucket/_service.py b/src/aignostics/bucket/_service.py index 87d0b65f1..ab5de0301 100644 --- a/src/aignostics/bucket/_service.py +++ b/src/aignostics/bucket/_service.py @@ -93,7 +93,7 @@ def __init__(self) -> None: super().__init__(Settings) self._platform_service = PlatformService() - def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 """Determine info of this service. Args: @@ -104,7 +104,7 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PL """ return {} - def health(self) -> Health: # noqa: PLR6301 + async def health(self) -> Health: # noqa: PLR6301 """Determine health of this service. Returns: diff --git a/src/aignostics/dataset/_service.py b/src/aignostics/dataset/_service.py index f013037ca..1d1e07da1 100644 --- a/src/aignostics/dataset/_service.py +++ b/src/aignostics/dataset/_service.py @@ -62,7 +62,7 @@ def _cleanup_processes() -> None: class Service(BaseService): """Service of the IDC module.""" - def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 """Determine info of this service. Args: @@ -73,7 +73,7 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PL """ return {} - def health(self) -> Health: # noqa: PLR6301 + async def health(self) -> Health: # noqa: PLR6301 """Determine health of hello service. Returns: diff --git a/src/aignostics/gui/_frame.py b/src/aignostics/gui/_frame.py index f4470f5a9..d7b4aa310 100644 --- a/src/aignostics/gui/_frame.py +++ b/src/aignostics/gui/_frame.py @@ -204,7 +204,7 @@ def health_link() -> None: async def _health_load_and_render() -> None: nonlocal launchpad_healthy with contextlib.suppress(Exception): - launchpad_healthy = bool(await run.cpu_bound(SystemService.health_static)) + launchpad_healthy = bool(await SystemService.health_static()) health_icon.refresh() health_link.refresh() diff --git a/src/aignostics/notebook/_service.py b/src/aignostics/notebook/_service.py index cc8dc0083..317f5c2f2 100644 --- a/src/aignostics/notebook/_service.py +++ b/src/aignostics/notebook/_service.py @@ -245,7 +245,7 @@ def _get_runner() -> _Runner: class Service(BaseService): """Service of the Marimo module.""" - def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 """Determine info of this service. Args: @@ -256,7 +256,7 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PL """ return {} - def health(self) -> Health: # noqa: PLR6301 + async def health(self) -> Health: # noqa: PLR6301 """Determine health of hello service. Returns: diff --git a/src/aignostics/platform/_service.py b/src/aignostics/platform/_service.py index b1fb57eb9..40f10a643 100644 --- a/src/aignostics/platform/_service.py +++ b/src/aignostics/platform/_service.py @@ -171,7 +171,7 @@ def _get_http_pool(cls) -> urllib3.PoolManager: ) return cls._http_pool - def info(self, mask_secrets: bool = True) -> dict[str, Any]: + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: """Determine info of this service. Args: @@ -278,7 +278,7 @@ def _determine_api_authenticated_health(self) -> Health: logger.exception("Issue with Aignostics Platform API") return Health(status=Health.Code.DOWN, reason=f"Issue with Aignostics Platform API: '{e}'") - def health(self) -> Health: + async def health(self) -> Health: """Determine health of this service. Returns: diff --git a/src/aignostics/qupath/_service.py b/src/aignostics/qupath/_service.py index d8d9b6739..a656a91ea 100644 --- a/src/aignostics/qupath/_service.py +++ b/src/aignostics/qupath/_service.py @@ -209,7 +209,7 @@ def __init__(self) -> None: """Initialize service.""" super().__init__(Settings) - def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 """Determine info of this service. Args: @@ -228,7 +228,7 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PL } } - def health(self) -> Health: # noqa: PLR6301 + async def health(self) -> Health: # noqa: PLR6301 """Determine health of this service. Returns: diff --git a/src/aignostics/system/_cli.py b/src/aignostics/system/_cli.py index f1b201a46..836e648cb 100644 --- a/src/aignostics/system/_cli.py +++ b/src/aignostics/system/_cli.py @@ -1,5 +1,6 @@ """System CLI commands.""" +import asyncio import json import sys from enum import StrEnum @@ -51,7 +52,7 @@ def health( Args: output_format (OutputFormat): Output format (JSON or YAML). """ - health = _service.health() + health = asyncio.run(_service.health()) match output_format: case OutputFormat.JSON: console.print_json(data=health.model_dump()) @@ -79,7 +80,7 @@ def info( mask_secrets (bool): Mask values for variables identified as secrets. output_format (OutputFormat): Output format (JSON or YAML). """ - info = _service.info(include_environ=include_environ, mask_secrets=mask_secrets) + info = asyncio.run(_service.info(include_environ=include_environ, mask_secrets=mask_secrets)) match output_format: case OutputFormat.JSON: console.print_json(data=info) diff --git a/src/aignostics/system/_gui.py b/src/aignostics/system/_gui.py index 84a83526c..31a33c341 100644 --- a/src/aignostics/system/_gui.py +++ b/src/aignostics/system/_gui.py @@ -12,7 +12,7 @@ class PageBuilder(BasePageBuilder): @staticmethod def register_pages() -> None: # noqa: PLR0915 - from nicegui import app, run, ui # noqa: PLC0415 + from nicegui import app, ui # noqa: PLC0415 locate_subclasses(BaseService) # Ensure settings are loaded app.add_static_files("/system_assets", Path(__file__).parent / "assets") @@ -49,7 +49,7 @@ async def page_system() -> None: # noqa: PLR0915 } editor = ui.json_editor(properties).style("width: 100%").mark("JSON_EDITOR_HEALTH") editor.set_visibility(False) - health = await run.cpu_bound(Service.health_static) + health = await Service.health_static() if health is None: properties["content"] = {"json": "Health check failed."} # type: ignore[unreachable] else: @@ -84,9 +84,7 @@ async def load_info(mask_secrets: bool = True) -> None: editor.set_visibility(False) spinner.set_visibility(True) mask_secrets_switch.set_visibility(False) - info = await run.cpu_bound( - Service.info, include_environ=True, mask_secrets=mask_secrets - ) + info = await Service.info(include_environ=True, mask_secrets=mask_secrets) if info is None: properties["content"] = {"json": "Info retrieval failed."} # type: ignore[unreachable] else: diff --git a/src/aignostics/system/_service.py b/src/aignostics/system/_service.py index a4bf0ca4e..11117f05a 100644 --- a/src/aignostics/system/_service.py +++ b/src/aignostics/system/_service.py @@ -134,7 +134,7 @@ def _determine_network_health() -> Health: return Health(status=Health.Code.UP) @staticmethod - def health_static() -> Health: + async def health_static() -> Health: """Determine health of the system. - This method is static and does not require an instance of the service. @@ -143,9 +143,9 @@ def health_static() -> Health: Returns: Health: The health of the system. """ - return Service().health() + return await Service().health() - def health(self) -> Health: + async def health(self) -> Health: """Determine aggregate health of the system. - Health exposed by implementations of BaseService in other @@ -158,7 +158,7 @@ def health(self) -> Health: components: dict[str, Health] = {} for service_class in locate_subclasses(BaseService): if service_class is not Service: - components[f"{service_class.__module__}.{service_class.__name__}"] = service_class().health() + components[f"{service_class.__module__}.{service_class.__name__}"] = await service_class().health() components["network"] = self._determine_network_health() # Set the system health status based on is_healthy attribute @@ -291,7 +291,7 @@ def _collect_all_settings(mask_secrets: bool = True) -> dict[str, Any]: return {k: settings[k] for k in sorted(settings)} @staticmethod - def info(include_environ: bool = False, mask_secrets: bool = True) -> dict[str, Any]: # type: ignore[override] + async def info(include_environ: bool = False, mask_secrets: bool = True) -> dict[str, Any]: # type: ignore[override] """ Get info about configuration of service. @@ -417,7 +417,7 @@ def info(include_environ: bool = False, mask_secrets: bool = True) -> dict[str, for service_class in locate_subclasses(BaseService): if service_class is not Service: service = service_class() - result_dict[service.key()] = service.info(mask_secrets=mask_secrets) + result_dict[service.key()] = await service.info(mask_secrets=mask_secrets) logger.debug("Service info: {}", result_dict) return result_dict diff --git a/src/aignostics/utils/CLAUDE.md b/src/aignostics/utils/CLAUDE.md index bb57b1174..e5cc2fa6e 100644 --- a/src/aignostics/utils/CLAUDE.md +++ b/src/aignostics/utils/CLAUDE.md @@ -59,13 +59,38 @@ subclasses = locate_subclasses(BaseService) # Services inherit from BaseService class MyService(BaseService): - def health(self) -> Health: + async def health(self) -> Health: return Health(status=Health.Code.UP) - def info(self, mask_secrets=True) -> dict: + async def info(self, mask_secrets=True) -> dict: return {"version": "1.0.0"} ``` +**FastAPI Dependency Injection:** + +`BaseService.get_service()` returns a cached FastAPI dependency function that yields a service instance. +The same function object is returned on repeated calls (required for `dependency_overrides` in tests). + +```python +from typing import Annotated +from fastapi import Depends +from aignostics.my_module._service import Service + +@router.get("/endpoint") +async def endpoint(service: Annotated[Service, Depends(Service.get_service())]): + return service.do_something() +``` + +**Settings Accessor:** + +`BaseService.settings()` exposes `self._settings` as a public method for callers that need access +to the service's configuration object. + +```python +service = MyService() +settings = service.settings() # Returns the BaseSettings instance +``` + **User Agent Generation:** ```python @@ -114,7 +139,7 @@ settings = load_settings(MySettings) from aignostics.utils import Health, BaseService class MyService(BaseService): - def health(self) -> Health: + async def health(self) -> Health: return Health( status=Health.Code.UP, details={"database": "connected"} diff --git a/src/aignostics/utils/_service.py b/src/aignostics/utils/_service.py index 819d96cd0..e9d5eb5ef 100644 --- a/src/aignostics/utils/_service.py +++ b/src/aignostics/utils/_service.py @@ -1,7 +1,8 @@ """Base class for services.""" from abc import ABC, abstractmethod -from typing import Any, TypeVar +from collections.abc import Callable, Generator +from typing import Any, ClassVar, Self, TypeVar, cast from pydantic_settings import BaseSettings @@ -15,6 +16,7 @@ class BaseService(ABC): """Base class for services.""" _settings: BaseSettings + _cached_dependency: ClassVar[Callable[[], Generator[Any]] | None] = None def __init__(self, settings_class: type[T] | None = None) -> None: """ @@ -26,12 +28,51 @@ def __init__(self, settings_class: type[T] | None = None) -> None: if settings_class is not None: self._settings = load_settings(settings_class) + @classmethod + def get_service(cls) -> Callable[[], Generator[Self]]: + """Create a FastAPI dependency that yields an instance of this service. + + This is a factory method that returns a cached dependency function suitable + for use with FastAPI's Depends(). It eliminates boilerplate code by providing + a standard pattern for service injection. + + The dependency function is cached per-class to ensure the same function object + is returned each time, which is necessary for FastAPI's dependency_overrides + to work correctly in tests. + + Returns: + Callable: A dependency function that yields the service instance. + + Example: + ```python + from aignostics.my_module._service import Service + + + @router.get("/endpoint") + async def endpoint(service: Annotated[Service, Depends(Service.get_service())]): + return await service.do_something() + ``` + """ + # Check if this specific class already has a cached dependency. + # We use a class-specific cache key to avoid inheritance issues. + cache_attr = f"_cached_dependency_{cls.__name__}" + cached = getattr(cls, cache_attr, None) + if cached is not None: + return cast("Callable[[], Generator[Self]]", cached) + + def dependency() -> Generator[Self]: + service = cls() + yield service + + setattr(cls, cache_attr, dependency) + return dependency + def key(self) -> str: """Return the module name of the instance.""" return self.__module__.split(".")[-2] @abstractmethod - def health(self) -> Health: + async def health(self) -> Health: """Get health of this service. Override in subclass. Returns: @@ -39,7 +80,7 @@ def health(self) -> Health: """ @abstractmethod - def info(self, mask_secrets: bool = True) -> dict[str, Any]: + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: """Get info of this service. Override in subclass. Args: @@ -48,3 +89,11 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: Returns: dict[str, Any]: Information about the service. """ + + def settings(self) -> BaseSettings: + """Get the settings of this service. + + Returns: + BaseSettings: The settings of the service. + """ + return self._settings diff --git a/src/aignostics/wsi/_service.py b/src/aignostics/wsi/_service.py index 702ace1fb..358cde04b 100644 --- a/src/aignostics/wsi/_service.py +++ b/src/aignostics/wsi/_service.py @@ -20,7 +20,7 @@ class Service(BaseService): """Service of the application module.""" - def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PLR6301 """Determine info of this service. Args: @@ -31,7 +31,7 @@ def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: ARG002, PL """ return {} - def health(self) -> Health: # noqa: PLR6301 + async def health(self) -> Health: # noqa: PLR6301 """Determine health of thumbnail service. Returns: diff --git a/tests/aignostics/platform/service_test.py b/tests/aignostics/platform/service_test.py index 33a42792e..f3d809361 100644 --- a/tests/aignostics/platform/service_test.py +++ b/tests/aignostics/platform/service_test.py @@ -211,7 +211,7 @@ def test_determine_api_authenticated_health_degraded_response() -> None: @pytest.mark.unit -def test_health_returns_both_components() -> None: +async def test_health_returns_both_components() -> None: """health() aggregates api_public and api_authenticated component keys.""" public_health = Health(status=Health.Code.UP) auth_health = Health(status=Health.Code.UP) @@ -221,7 +221,7 @@ def test_health_returns_both_components() -> None: patch.object(service, "_determine_api_public_health", return_value=public_health), patch.object(service, "_determine_api_authenticated_health", return_value=auth_health), ): - result = service.health() + result = await service.health() assert result.components is not None assert "api_public" in result.components diff --git a/tests/aignostics/utils/service_test.py b/tests/aignostics/utils/service_test.py new file mode 100644 index 000000000..8ffa02e9e --- /dev/null +++ b/tests/aignostics/utils/service_test.py @@ -0,0 +1,123 @@ +"""Tests for BaseService.get_service() and settings() methods.""" + +import contextlib +import inspect +from typing import Any + +import pytest + +from aignostics.utils._health import Health +from aignostics.utils._service import BaseService + + +class _ConcreteService(BaseService): + """Minimal concrete service for testing BaseService behaviour.""" + + async def health(self) -> Health: # noqa: PLR6301 + return Health(status=Health.Code.UP) + + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: PLR6301 + return {} + + +class _AnotherConcreteService(BaseService): + """A second concrete service to test per-class caching.""" + + async def health(self) -> Health: # noqa: PLR6301 + return Health(status=Health.Code.UP) + + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: PLR6301 + return {} + + +@pytest.fixture(autouse=True) +def _clear_cached_dependencies() -> None: + """Remove any cached dependency attributes between tests.""" + for cls in (_ConcreteService, _AnotherConcreteService): + cache_attr = f"_cached_dependency_{cls.__name__}" + if hasattr(cls, cache_attr): + delattr(cls, cache_attr) + + +@pytest.mark.unit +def test_get_service_returns_callable(record_property) -> None: + """get_service() returns a callable dependency factory.""" + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + dep = _ConcreteService.get_service() + assert callable(dep) + + +@pytest.mark.unit +def test_get_service_caching_returns_same_object(record_property) -> None: + """Calling get_service() twice on the same class returns the identical function. + + FastAPI's dependency_overrides keying relies on object identity. + """ + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + dep1 = _ConcreteService.get_service() + dep2 = _ConcreteService.get_service() + assert dep1 is dep2 + + +@pytest.mark.unit +def test_get_service_caching_is_per_class(record_property) -> None: + """Two different subclasses receive different dependency functions.""" + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + dep_a = _ConcreteService.get_service() + dep_b = _AnotherConcreteService.get_service() + assert dep_a is not dep_b + + +@pytest.mark.unit +def test_get_service_dependency_yields_instance(record_property) -> None: + """Exhausting the dependency generator yields an instance of the service class.""" + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + dep = _ConcreteService.get_service() + gen = dep() + instance = next(gen) + assert isinstance(instance, _ConcreteService) + with contextlib.suppress(StopIteration): + next(gen) + + +@pytest.mark.unit +def test_settings_accessor_returns_settings(record_property) -> None: + """settings() returns the _settings object set during __init__.""" + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + from pydantic_settings import BaseSettings as _BaseSettings + + class _MinimalSettings(_BaseSettings): + pass + + class _ServiceWithSettings(BaseService): + def __init__(self) -> None: + super().__init__(_MinimalSettings) + + async def health(self) -> Health: # noqa: PLR6301 + return Health(status=Health.Code.UP) + + async def info(self, mask_secrets: bool = True) -> dict[str, Any]: # noqa: PLR6301 + return {} + + svc = _ServiceWithSettings() + result = svc.settings() + assert isinstance(result, _MinimalSettings) + assert result is svc._settings + + +@pytest.mark.unit +def test_health_is_coroutine(record_property) -> None: + """health() returns a coroutine when called.""" + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + coro = _ConcreteService().health() + assert inspect.iscoroutine(coro) + coro.close() + + +@pytest.mark.unit +def test_info_is_coroutine(record_property) -> None: + """info() returns a coroutine when called.""" + record_property("tested-item-id", "SPEC-UTILS-SERVICE") + coro = _ConcreteService().info() + assert inspect.iscoroutine(coro) + coro.close() From 14f6830951a9b2eda88ba0813496475569b025f0 Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 19 Mar 2026 12:31:23 +0100 Subject: [PATCH 2/6] perf(system): make system info and health async --- src/aignostics/system/_service.py | 72 +++++++------------ tests/aignostics/system/cli_test.py | 8 +-- .../aignostics/system/service_pooling_test.py | 43 ----------- 3 files changed, 31 insertions(+), 92 deletions(-) delete mode 100644 tests/aignostics/system/service_pooling_test.py diff --git a/src/aignostics/system/_service.py b/src/aignostics/system/_service.py index 11117f05a..c58c69089 100644 --- a/src/aignostics/system/_service.py +++ b/src/aignostics/system/_service.py @@ -1,5 +1,6 @@ """System service.""" +import asyncio import json import os import platform @@ -10,10 +11,10 @@ from http import HTTPStatus from pathlib import Path from socket import AF_INET, SOCK_DGRAM, socket -from typing import Any, ClassVar, NotRequired, TypedDict +from typing import Any, NotRequired, TypedDict from urllib.request import getproxies -import urllib3 +import httpx from dotenv import set_key as dotenv_set_key from dotenv import unset_key as dotenv_unset_key from loguru import logger @@ -70,28 +71,11 @@ class Service(BaseService): """System service.""" _settings: Settings - _http_pool: ClassVar[urllib3.PoolManager | None] = None def __init__(self) -> None: """Initialize service.""" super().__init__(Settings) - @classmethod - def _get_http_pool(cls) -> urllib3.PoolManager: - """Get or create the shared HTTP pool manager. - - All service instances share the same urllib3.PoolManager for efficient connection reuse. - - Returns: - urllib3.PoolManager: Shared connection pool manager. - """ - if cls._http_pool is None: - cls._http_pool = urllib3.PoolManager( - maxsize=10, # Max connections per host - block=False, # Don't block if pool is full - ) - return cls._http_pool - @staticmethod def _is_healthy() -> bool: """Check if the service itself is healthy. @@ -102,29 +86,28 @@ def _is_healthy() -> bool: return True @staticmethod - def _determine_network_health() -> Health: + async def _determine_network_health() -> Health: """Determine we can reach a well known and secure endpoint. - Checks if health endpoint is reachable and returns 200 OK - - Uses urllib3 for a direct connection check without authentication + - Uses httpx for an async connection check without authentication Returns: Health: The healthiness of the network connection via basic unauthenticated request. """ try: - http = Service._get_http_pool() - response = http.request( - method="GET", - url=IPIFY_URL, - headers={"User-Agent": user_agent()}, - timeout=urllib3.Timeout(total=NETWORK_TIMEOUT), - ) + async with httpx.AsyncClient() as client: + response = await client.get( + IPIFY_URL, + headers={"User-Agent": user_agent()}, + timeout=NETWORK_TIMEOUT, + ) - if response.status != HTTPStatus.OK: - logger.error(f"'{IPIFY_URL}' returned '{response.status}'") + if response.status_code != HTTPStatus.OK: + logger.error(f"'{IPIFY_URL}' returned '{response.status_code}'") return Health( status=Health.Code.DOWN, - reason=f"'{IPIFY_URL}' returned status '{response.status}'", + reason=f"'{IPIFY_URL}' returned status '{response.status_code}'", ) except Exception as e: message = f"Issue reaching {IPIFY_URL}: {e}" @@ -159,7 +142,7 @@ async def health(self) -> Health: for service_class in locate_subclasses(BaseService): if service_class is not Service: components[f"{service_class.__module__}.{service_class.__name__}"] = await service_class().health() - components["network"] = self._determine_network_health() + components["network"] = await self._determine_network_health() # Set the system health status based on is_healthy attribute status = Health.Code.UP if self._is_healthy() else Health.Code.DOWN @@ -178,7 +161,7 @@ def is_token_valid(self, token: str) -> bool: return token == self._settings.token.get_secret_value() @staticmethod - def _get_public_ipv4(timeout: int = NETWORK_TIMEOUT) -> str | None: + async def _get_public_ipv4(timeout: int = NETWORK_TIMEOUT) -> str | None: """Get the public IPv4 address of the system. Args: @@ -188,16 +171,12 @@ def _get_public_ipv4(timeout: int = NETWORK_TIMEOUT) -> str | None: str | None: The public IPv4 address, or None if failed. """ try: - http = Service._get_http_pool() - response = http.request( - method="GET", - url=IPIFY_URL, - timeout=urllib3.Timeout(total=timeout), - ) - if response.status != HTTPStatus.OK: - logger.error(f"Failed to get public IP: HTTP {response.status}") + async with httpx.AsyncClient() as client: + response = await client.get(IPIFY_URL, timeout=timeout) + if response.status_code != HTTPStatus.OK: + logger.error(f"Failed to get public IP: HTTP {response.status_code}") return None - return response.data.decode("utf-8") + return response.text except Exception as e: message = f"Failed to get public IP: {e}" logger.exception(message) @@ -314,8 +293,11 @@ async def info(include_environ: bool = False, mask_secrets: bool = True) -> dict bootdatetime = boottime() vmem = psutil.virtual_memory() swap = psutil.swap_memory() - cpu_percent = psutil.cpu_percent(interval=MEASURE_INTERVAL_SECONDS) - cpu_times_percent = psutil.cpu_times_percent(interval=MEASURE_INTERVAL_SECONDS) + psutil.cpu_percent(interval=None) # prime the counter + psutil.cpu_times_percent(interval=None) # prime the counter + await asyncio.sleep(MEASURE_INTERVAL_SECONDS) + cpu_percent = psutil.cpu_percent(interval=None) + cpu_times_percent = psutil.cpu_times_percent(interval=None) cpu_freq = None try: cpu_freq = psutil.cpu_freq() if hasattr(psutil, "cpu_freq") else None # Happens on macOS latest VM on GHA @@ -377,7 +359,7 @@ async def info(include_environ: bool = False, mask_secrets: bool = True) -> dict "network": { "hostname": platform.node(), "local_ipv4": Service._get_local_ipv4(), - "public_ipv4": Service._get_public_ipv4(), + "public_ipv4": await Service._get_public_ipv4(), "proxies": getproxies(), "requests_ca_bundle": os.getenv("REQUESTS_CA_BUNDLE"), "ssl_cert_file": os.getenv("SSL_CERT_FILE"), diff --git a/tests/aignostics/system/cli_test.py b/tests/aignostics/system/cli_test.py index 74aef4dd1..4b621b11f 100644 --- a/tests/aignostics/system/cli_test.py +++ b/tests/aignostics/system/cli_test.py @@ -3,7 +3,7 @@ import logging import os from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest from typer.testing import CliRunner @@ -90,7 +90,7 @@ def test_cli_health_up_exits_zero(mock_service: MagicMock, runner: CliRunner) -> """Check health command exits with code 0 when status is UP.""" from aignostics.utils import Health - mock_service.health.return_value = Health(status=Health.Code.UP) + mock_service.health = AsyncMock(return_value=Health(status=Health.Code.UP)) result = runner.invoke(cli, ["system", "health"]) assert result.exit_code == 0 @@ -101,7 +101,7 @@ def test_cli_health_degraded_exits_zero(mock_service: MagicMock, runner: CliRunn """Check health command exits with code 0 when status is DEGRADED.""" from aignostics.utils import Health - mock_service.health.return_value = Health(status=Health.Code.DEGRADED, reason="some component degraded") + mock_service.health = AsyncMock(return_value=Health(status=Health.Code.DEGRADED, reason="some component degraded")) result = runner.invoke(cli, ["system", "health"]) assert result.exit_code == 0 @@ -112,7 +112,7 @@ def test_cli_health_down_exits_one(mock_service: MagicMock, runner: CliRunner) - """Check health command exits with code 1 when status is DOWN.""" from aignostics.utils import Health - mock_service.health.return_value = Health(status=Health.Code.DOWN, reason="service unavailable") + mock_service.health = AsyncMock(return_value=Health(status=Health.Code.DOWN, reason="service unavailable")) result = runner.invoke(cli, ["system", "health"]) assert result.exit_code == 1 diff --git a/tests/aignostics/system/service_pooling_test.py b/tests/aignostics/system/service_pooling_test.py deleted file mode 100644 index bff8d2e79..000000000 --- a/tests/aignostics/system/service_pooling_test.py +++ /dev/null @@ -1,43 +0,0 @@ -"""Tests for system service connection pooling.""" - -import pytest - -from aignostics.system._service import Service - - -@pytest.mark.unit -def test_http_pool_is_shared(record_property) -> None: - """Test that Service._get_http_pool returns the same instance across multiple calls. - - This ensures that all service instances share the same urllib3.PoolManager - for efficient connection reuse when calling ipify. - """ - # Get pool instance - record_property("tested-item-id", "SPEC-SYSTEM-SERVICE") - pool1 = Service._get_http_pool() - - # Get pool instance again (should return same instance) - pool2 = Service._get_http_pool() - - # Verify both calls return the same instance - assert pool1 is pool2, "Service._get_http_pool should return the same PoolManager instance" - - -@pytest.mark.unit -def test_http_pool_singleton(record_property) -> None: - """Test that Service._http_pool maintains a singleton pattern. - - Multiple service instances should share the same connection pool. - """ - record_property("tested-item-id", "SPEC-SYSTEM-SERVICE") - - # Create two service instances - service1 = Service() - service2 = Service() - - # Get pool from each service's perspective - pool_from_service1 = service1._get_http_pool() - pool_from_service2 = service2._get_http_pool() - - # Verify they share the same pool - assert pool_from_service1 is pool_from_service2, "Service instances should share the same HTTP pool" From a47e18a8277d86d8d6bd553c35c608992484d49d Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 19 Mar 2026 15:30:33 +0100 Subject: [PATCH 3/6] fix(test): run CLI invoke in thread from async test --- tests/aignostics/application/gui_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/aignostics/application/gui_test.py b/tests/aignostics/application/gui_test.py index 5231bc6cd..763c9a613 100644 --- a/tests/aignostics/application/gui_test.py +++ b/tests/aignostics/application/gui_test.py @@ -3,7 +3,7 @@ import contextlib import re import tempfile -from asyncio import sleep +from asyncio import sleep, to_thread from datetime import UTC, datetime, timedelta from pathlib import Path from typing import TYPE_CHECKING @@ -101,7 +101,8 @@ async def test_gui_cli_submit_to_run_result_delete( csv_content += ";5onqtA==;0.26268186053789266;7447;7196;H&E;LUNG;LUNG_CANCER;gs://bucket/test" csv_path = tmp_path / "dummy.csv" csv_path.write_text(csv_content) - result = runner.invoke( + result = await to_thread( + runner.invoke, cli, [ "application", From 1260db1a3b58c8c0d4d8ba1a0bdc3ac06410f234 Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 19 Mar 2026 15:37:37 +0100 Subject: [PATCH 4/6] refactor(platform): replace urllib3 pool manager with httpx in health checks Co-Authored-By: Claude Sonnet 4.6 --- src/aignostics/platform/_service.py | 75 ++++----- tests/aignostics/platform/service_test.py | 189 ++++++++++------------ 2 files changed, 113 insertions(+), 151 deletions(-) diff --git a/src/aignostics/platform/_service.py b/src/aignostics/platform/_service.py index 40f10a643..50bb409c5 100644 --- a/src/aignostics/platform/_service.py +++ b/src/aignostics/platform/_service.py @@ -6,7 +6,7 @@ from http import HTTPStatus from typing import Any -import urllib3 +import httpx from aignx.codegen.models import MeReadResponse as Me from aignx.codegen.models import OrganizationReadResponse as Organization from aignx.codegen.models import UserReadResponse as User @@ -149,28 +149,11 @@ class Service(BaseService): """Service of the application module.""" _settings: Settings - _http_pool: urllib3.PoolManager | None = None def __init__(self) -> None: """Initialize service.""" super().__init__(Settings) # automatically loads and validates the settings - @classmethod - def _get_http_pool(cls) -> urllib3.PoolManager: - """Get or create the shared HTTP pool manager. - - All service instances share the same urllib3.PoolManager for efficient connection reuse. - - Returns: - urllib3.PoolManager: Shared connection pool manager. - """ - if cls._http_pool is None: - cls._http_pool = urllib3.PoolManager( - maxsize=10, # Max connections per host - block=False, # Don't block if pool is full - ) - return cls._http_pool - async def info(self, mask_secrets: bool = True) -> dict[str, Any]: """Determine info of this service. @@ -193,26 +176,26 @@ async def info(self, mask_secrets: bool = True) -> dict[str, Any]: } @staticmethod - def _health_from_response(response: urllib3.BaseHTTPResponse) -> Health: + def _health_from_response(response: httpx.Response) -> Health: """Map a PAPI health response to a Health status. Handles non-200 status codes, unparseable bodies, and the three recognised ``status`` values (``"UP"``, ``"DEGRADED"``, ``"DOWN"``). Args: - response: urllib3 response from the ``/health`` endpoint. + response: httpx response from the ``/health`` endpoint. Returns: Health: ``UP``, ``DEGRADED``, or ``DOWN`` derived from the response. """ - if response.status != HTTPStatus.OK: - logger.error("Aignostics Platform API returned '{}'", response.status) + if response.status_code != HTTPStatus.OK: + logger.error("Aignostics Platform API returned '{}'", response.status_code) return Health( - status=Health.Code.DOWN, reason=f"Aignostics Platform API returned status '{response.status}'" + status=Health.Code.DOWN, reason=f"Aignostics Platform API returned status '{response.status_code}'" ) try: - body = json.loads(response.data) + body = response.json() except Exception: return Health(status=Health.Code.DOWN, reason="Aignostics Platform API returned unparseable response") @@ -228,34 +211,31 @@ def _health_from_response(response: urllib3.BaseHTTPResponse) -> Health: reason=f"Aignostics Platform API returned unknown status '{api_status}'", ) - def _determine_api_public_health(self) -> Health: + async def _determine_api_public_health(self) -> Health: """Determine healthiness and reachability of Aignostics Platform API. - Checks if health endpoint is reachable and returns 200 OK - Parses the response body to detect DEGRADED status - - Uses urllib3 for a direct connection check without authentication + - Uses httpx for a direct connection check without authentication Returns: Health: The healthiness of the Aignostics Platform API via basic unauthenticated request. """ try: - http = self._get_http_pool() - response = http.request( - method="GET", - url=f"{self._settings.api_root}/health", - headers={"User-Agent": user_agent()}, - timeout=urllib3.Timeout(total=self._settings.health_timeout), - ) + async with httpx.AsyncClient() as client: + response = await client.get( + f"{self._settings.api_root}/health", + headers={"User-Agent": user_agent()}, + timeout=self._settings.health_timeout, + ) return self._health_from_response(response) except Exception as e: logger.exception("Issue with Aignostics Platform API") return Health(status=Health.Code.DOWN, reason=f"Issue with Aignostics Platform API: '{e}'") - def _determine_api_authenticated_health(self) -> Health: + async def _determine_api_authenticated_health(self) -> Health: """Determine healthiness and reachability of Aignostics Platform API via authenticated request. - Uses a dedicated HTTP pool (separate from the API client's connection pool) to prevent - connection-level cross-contamination between health checks and API calls. Parses the response body to detect DEGRADED status. Returns: @@ -263,16 +243,15 @@ def _determine_api_authenticated_health(self) -> Health: """ try: token = get_token(use_cache=True) - http = self._get_http_pool() - response = http.request( - method="GET", - url=f"{self._settings.api_root}/health", - headers={ - "User-Agent": user_agent(), - "Authorization": f"Bearer {token}", - }, - timeout=urllib3.Timeout(total=self._settings.health_timeout), - ) + async with httpx.AsyncClient() as client: + response = await client.get( + f"{self._settings.api_root}/health", + headers={ + "User-Agent": user_agent(), + "Authorization": f"Bearer {token}", + }, + timeout=self._settings.health_timeout, + ) return self._health_from_response(response) except Exception as e: logger.exception("Issue with Aignostics Platform API") @@ -287,8 +266,8 @@ async def health(self) -> Health: return Health( status=Health.Code.UP, components={ - "api_public": self._determine_api_public_health(), - "api_authenticated": self._determine_api_authenticated_health(), + "api_public": await self._determine_api_public_health(), + "api_authenticated": await self._determine_api_authenticated_health(), }, ) diff --git a/tests/aignostics/platform/service_test.py b/tests/aignostics/platform/service_test.py index f3d809361..c1dbeea65 100644 --- a/tests/aignostics/platform/service_test.py +++ b/tests/aignostics/platform/service_test.py @@ -1,7 +1,7 @@ """Tests for the platform service module.""" from http import HTTPStatus -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -12,199 +12,182 @@ @pytest.mark.unit -def test_http_pool_is_shared() -> None: - """Test that Service._get_http_pool returns the same instance across multiple calls. - - This ensures that all service instances share the same urllib3.PoolManager - for efficient connection reuse. - """ - # Get pool instance - pool1 = Service._get_http_pool() - - # Get pool instance again (should return same instance) - pool2 = Service._get_http_pool() - - # Verify both calls return the same instance - assert pool1 is pool2, "Service._get_http_pool should return the same PoolManager instance" - - -@pytest.mark.unit -def test_http_pool_singleton() -> None: - """Test that Service._http_pool maintains a singleton pattern. - - Multiple service instances should share the same connection pool. - """ - # Create two service instances - service1 = Service() - service2 = Service() - - # Get pool from each service's perspective - pool_from_service1 = service1._get_http_pool() - pool_from_service2 = service2._get_http_pool() - - # Verify they share the same pool - assert pool_from_service1 is pool_from_service2, "Service instances should share the same HTTP pool" - - -@pytest.mark.unit -def test_determine_api_authenticated_health_success() -> None: - """Health.UP returned when the dedicated pool responds 200 with auth token.""" +async def test_determine_api_authenticated_health_success() -> None: + """Health.UP returned when httpx responds 200 with auth token.""" mock_response = MagicMock() - mock_response.status = HTTPStatus.OK - mock_response.data = b'{"status": "UP"}' + mock_response.status_code = HTTPStatus.OK + mock_response.json.return_value = {"status": "UP"} - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response with ( - patch.object(Service, "_get_http_pool", return_value=mock_pool), + patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls, patch(_PATCH_AUTH_GETTER, return_value="test-token"), ): - result = Service()._determine_api_authenticated_health() + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_authenticated_health() assert result.status == Health.Code.UP @pytest.mark.unit -def test_determine_api_authenticated_health_non_200() -> None: - """Health.DOWN returned when the dedicated pool responds with non-200.""" +async def test_determine_api_authenticated_health_non_200() -> None: + """Health.DOWN returned when httpx responds with non-200.""" mock_response = MagicMock() - mock_response.status = HTTPStatus.SERVICE_UNAVAILABLE + mock_response.status_code = HTTPStatus.SERVICE_UNAVAILABLE - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response with ( - patch.object(Service, "_get_http_pool", return_value=mock_pool), + patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls, patch(_PATCH_AUTH_GETTER, return_value="test-token"), ): - result = Service()._determine_api_authenticated_health() + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_authenticated_health() assert result.status == Health.Code.DOWN assert result.reason is not None @pytest.mark.unit -def test_determine_api_authenticated_health_handles_exception() -> None: +async def test_determine_api_authenticated_health_handles_exception() -> None: """Health.DOWN with reason when get_token raises.""" with patch(_PATCH_AUTH_GETTER, side_effect=RuntimeError("no auth")): - result = Service()._determine_api_authenticated_health() + result = await Service()._determine_api_authenticated_health() assert result.status == Health.Code.DOWN assert result.reason is not None @pytest.mark.unit -def test_determine_api_public_health_non_200() -> None: - """Health.DOWN returned when the public pool responds with non-200.""" +async def test_determine_api_public_health_non_200() -> None: + """Health.DOWN returned when httpx responds with non-200.""" mock_response = MagicMock() - mock_response.status = HTTPStatus.SERVICE_UNAVAILABLE + mock_response.status_code = HTTPStatus.SERVICE_UNAVAILABLE - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response - with patch.object(Service, "_get_http_pool", return_value=mock_pool): - result = Service()._determine_api_public_health() + with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_public_health() assert result.status == Health.Code.DOWN assert result.reason is not None @pytest.mark.unit -def test_determine_api_public_health_handles_exception() -> None: - """Health.DOWN returned when the public pool raises.""" - mock_pool = MagicMock() - mock_pool.request.side_effect = ConnectionError("unreachable") +async def test_determine_api_public_health_handles_exception() -> None: + """Health.DOWN returned when httpx raises.""" + mock_client = AsyncMock() + mock_client.get.side_effect = ConnectionError("unreachable") - with patch.object(Service, "_get_http_pool", return_value=mock_pool): - result = Service()._determine_api_public_health() + with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_public_health() assert result.status == Health.Code.DOWN assert result.reason is not None @pytest.mark.unit -def test_determine_api_public_health_up_response() -> None: +async def test_determine_api_public_health_up_response() -> None: """HTTP 200 + {"status": "UP"} body → Health.UP (explicit JSON body check).""" mock_response = MagicMock() - mock_response.status = HTTPStatus.OK - mock_response.data = b'{"status": "UP"}' + mock_response.status_code = HTTPStatus.OK + mock_response.json.return_value = {"status": "UP"} - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response - with patch.object(Service, "_get_http_pool", return_value=mock_pool): - result = Service()._determine_api_public_health() + with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_public_health() assert result.status == Health.Code.UP @pytest.mark.unit -def test_determine_api_public_health_degraded_response() -> None: +async def test_determine_api_public_health_degraded_response() -> None: """HTTP 200 + {"status": "DEGRADED"} body → Health.DEGRADED with reason set.""" mock_response = MagicMock() - mock_response.status = HTTPStatus.OK - mock_response.data = b'{"status": "DEGRADED"}' + mock_response.status_code = HTTPStatus.OK + mock_response.json.return_value = {"status": "DEGRADED"} - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response - with patch.object(Service, "_get_http_pool", return_value=mock_pool): - result = Service()._determine_api_public_health() + with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_public_health() assert result.status == Health.Code.DEGRADED assert result.reason is not None @pytest.mark.unit -def test_determine_api_public_health_degraded_response_with_reason() -> None: +async def test_determine_api_public_health_degraded_response_with_reason() -> None: """HTTP 200 + {"status": "DEGRADED", "reason": "DB slow"} → reason == "DB slow".""" mock_response = MagicMock() - mock_response.status = HTTPStatus.OK - mock_response.data = b'{"status": "DEGRADED", "reason": "DB slow"}' + mock_response.status_code = HTTPStatus.OK + mock_response.json.return_value = {"status": "DEGRADED", "reason": "DB slow"} - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response - with patch.object(Service, "_get_http_pool", return_value=mock_pool): - result = Service()._determine_api_public_health() + with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_public_health() assert result.status == Health.Code.DEGRADED assert result.reason == "DB slow" @pytest.mark.unit -def test_determine_api_public_health_unknown_status_is_down() -> None: +async def test_determine_api_public_health_unknown_status_is_down() -> None: """HTTP 200 + {"status": "UNKNOWN"} body → Health.DOWN.""" mock_response = MagicMock() - mock_response.status = HTTPStatus.OK - mock_response.data = b'{"status": "UNKNOWN"}' + mock_response.status_code = HTTPStatus.OK + mock_response.json.return_value = {"status": "UNKNOWN"} - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response - with patch.object(Service, "_get_http_pool", return_value=mock_pool): - result = Service()._determine_api_public_health() + with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_public_health() assert result.status == Health.Code.DOWN assert result.reason is not None @pytest.mark.unit -def test_determine_api_authenticated_health_degraded_response() -> None: +async def test_determine_api_authenticated_health_degraded_response() -> None: """HTTP 200 + {"status": "DEGRADED"} body → Health.DEGRADED with reason set.""" mock_response = MagicMock() - mock_response.status = HTTPStatus.OK - mock_response.data = b'{"status": "DEGRADED"}' + mock_response.status_code = HTTPStatus.OK + mock_response.json.return_value = {"status": "DEGRADED"} - mock_pool = MagicMock() - mock_pool.request.return_value = mock_response + mock_client = AsyncMock() + mock_client.get.return_value = mock_response with ( - patch.object(Service, "_get_http_pool", return_value=mock_pool), + patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls, patch(_PATCH_AUTH_GETTER, return_value="test-token"), ): - result = Service()._determine_api_authenticated_health() + mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) + mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) + result = await Service()._determine_api_authenticated_health() assert result.status == Health.Code.DEGRADED assert result.reason is not None @@ -218,8 +201,8 @@ async def test_health_returns_both_components() -> None: service = Service() with ( - patch.object(service, "_determine_api_public_health", return_value=public_health), - patch.object(service, "_determine_api_authenticated_health", return_value=auth_health), + patch.object(service, "_determine_api_public_health", new=AsyncMock(return_value=public_health)), + patch.object(service, "_determine_api_authenticated_health", new=AsyncMock(return_value=auth_health)), ): result = await service.health() From 4e3c1deea0a4962390a3434b5addc1ad751548c7 Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 19 Mar 2026 16:32:35 +0100 Subject: [PATCH 5/6] refactor(system): extract _get_cpu_freq_info helper Extract CPU frequency measurement into a dedicated private static method to reduce cognitive complexity of Service.info() from 17 to 10. Co-Authored-By: Claude Sonnet 4.6 --- src/aignostics/system/_service.py | 33 ++++++++++++++-------- tests/aignostics/system/service_test.py | 37 +++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/aignostics/system/_service.py b/src/aignostics/system/_service.py index c58c69089..f532f02e0 100644 --- a/src/aignostics/system/_service.py +++ b/src/aignostics/system/_service.py @@ -269,6 +269,27 @@ def _collect_all_settings(mask_secrets: bool = True) -> dict[str, Any]: settings[flat_key] = value return {k: settings[k] for k in sorted(settings)} + @staticmethod + def _get_cpu_freq_info() -> dict[str, float | None]: + """Measure CPU frequency, handling platforms where it is unavailable. + + Returns: + dict[str, float | None]: A dict with keys ``current``, ``min``, ``max`` in MHz, + or ``None`` for each if the measurement is unavailable. + """ + import psutil # noqa: PLC0415 + + cpu_freq = None + try: + cpu_freq = psutil.cpu_freq() if hasattr(psutil, "cpu_freq") else None + except RuntimeError: + logger.warning("Failed to get CPU frequency.") # Happens on macOS VM on GHA + return { + "current": cpu_freq.current if cpu_freq else None, + "min": cpu_freq.min if cpu_freq else None, + "max": cpu_freq.max if cpu_freq else None, + } + @staticmethod async def info(include_environ: bool = False, mask_secrets: bool = True) -> dict[str, Any]: # type: ignore[override] """ @@ -298,12 +319,6 @@ async def info(include_environ: bool = False, mask_secrets: bool = True) -> dict await asyncio.sleep(MEASURE_INTERVAL_SECONDS) cpu_percent = psutil.cpu_percent(interval=None) cpu_times_percent = psutil.cpu_times_percent(interval=None) - cpu_freq = None - try: - cpu_freq = psutil.cpu_freq() if hasattr(psutil, "cpu_freq") else None # Happens on macOS latest VM on GHA - except RuntimeError: - logger.warning("Failed to get CPU frequency.") # Happens on macOS VM on GHA - rtn: InfoDict = { "package": { "version": __version__, @@ -336,11 +351,7 @@ async def info(include_environ: bool = False, mask_secrets: bool = True) -> dict "arch": platform.machine(), "processor": platform.processor(), "count": os.cpu_count(), - "frequency": { - "current": cpu_freq.current if cpu_freq else None, - "min": cpu_freq.min if cpu_freq else None, - "max": cpu_freq.max if cpu_freq else None, - }, + "frequency": Service._get_cpu_freq_info(), }, "memory": { "percent": vmem.percent, diff --git a/tests/aignostics/system/service_test.py b/tests/aignostics/system/service_test.py index abebfe1e0..2ae28c928 100644 --- a/tests/aignostics/system/service_test.py +++ b/tests/aignostics/system/service_test.py @@ -1,6 +1,7 @@ """Tests of the system service.""" import os +from typing import Any from unittest import mock import pytest @@ -8,6 +9,42 @@ from aignostics.system._service import Service +@pytest.mark.unit +def test_get_cpu_freq_info_returns_dict_with_expected_keys() -> None: + """Test that _get_cpu_freq_info returns a dict with exactly the keys current, min, max.""" + result = Service._get_cpu_freq_info() + assert set(result.keys()) == {"current", "min", "max"} + + +@pytest.mark.unit +def test_get_cpu_freq_info_handles_runtime_error() -> None: + """Test that a RuntimeError from psutil.cpu_freq is caught and all values are None.""" + import psutil + + with mock.patch.object(psutil, "cpu_freq", side_effect=RuntimeError("unavailable")): + result = Service._get_cpu_freq_info() + + assert result == {"current": None, "min": None, "max": None} + + +@pytest.mark.unit +def test_get_cpu_freq_info_handles_missing_cpu_freq() -> None: + """Test that a missing cpu_freq attribute on psutil is handled and all values are None.""" + import psutil + + had_cpu_freq = hasattr(psutil, "cpu_freq") + original: Any = getattr(psutil, "cpu_freq", None) + if had_cpu_freq: + delattr(psutil, "cpu_freq") + try: + result = Service._get_cpu_freq_info() + finally: + if had_cpu_freq: + psutil.cpu_freq = original # type: ignore[attr-defined] + + assert result == {"current": None, "min": None, "max": None} + + @pytest.mark.unit @pytest.mark.timeout(15) def test_is_token_valid(record_property) -> None: From dcc8028e826ea56100bae48d4f587819ca23a52a Mon Sep 17 00:00:00 2001 From: Oliver Meyer Date: Thu, 19 Mar 2026 16:33:49 +0100 Subject: [PATCH 6/6] refactor(test): extract constant for httpx patch path Replace 9 inline occurrences of the patch target string "aignostics.platform._service.httpx.AsyncClient" with a single module-level constant _PATCH_HTTPX_ASYNC_CLIENT, following the existing _PATCH_AUTH_GETTER convention. Co-Authored-By: Claude Sonnet 4.6 --- tests/aignostics/platform/service_test.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/aignostics/platform/service_test.py b/tests/aignostics/platform/service_test.py index c1dbeea65..74b0da3fd 100644 --- a/tests/aignostics/platform/service_test.py +++ b/tests/aignostics/platform/service_test.py @@ -9,6 +9,7 @@ from aignostics.utils import Health _PATCH_AUTH_GETTER = "aignostics.platform._service.get_token" +_PATCH_HTTPX_ASYNC_CLIENT = "aignostics.platform._service.httpx.AsyncClient" @pytest.mark.unit @@ -22,7 +23,7 @@ async def test_determine_api_authenticated_health_success() -> None: mock_client.get.return_value = mock_response with ( - patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls, + patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls, patch(_PATCH_AUTH_GETTER, return_value="test-token"), ): mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) @@ -42,7 +43,7 @@ async def test_determine_api_authenticated_health_non_200() -> None: mock_client.get.return_value = mock_response with ( - patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls, + patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls, patch(_PATCH_AUTH_GETTER, return_value="test-token"), ): mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) @@ -72,7 +73,7 @@ async def test_determine_api_public_health_non_200() -> None: mock_client = AsyncMock() mock_client.get.return_value = mock_response - with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + with patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls: mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) result = await Service()._determine_api_public_health() @@ -87,7 +88,7 @@ async def test_determine_api_public_health_handles_exception() -> None: mock_client = AsyncMock() mock_client.get.side_effect = ConnectionError("unreachable") - with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + with patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls: mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) result = await Service()._determine_api_public_health() @@ -106,7 +107,7 @@ async def test_determine_api_public_health_up_response() -> None: mock_client = AsyncMock() mock_client.get.return_value = mock_response - with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + with patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls: mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) result = await Service()._determine_api_public_health() @@ -124,7 +125,7 @@ async def test_determine_api_public_health_degraded_response() -> None: mock_client = AsyncMock() mock_client.get.return_value = mock_response - with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + with patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls: mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) result = await Service()._determine_api_public_health() @@ -143,7 +144,7 @@ async def test_determine_api_public_health_degraded_response_with_reason() -> No mock_client = AsyncMock() mock_client.get.return_value = mock_response - with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + with patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls: mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) result = await Service()._determine_api_public_health() @@ -162,7 +163,7 @@ async def test_determine_api_public_health_unknown_status_is_down() -> None: mock_client = AsyncMock() mock_client.get.return_value = mock_response - with patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls: + with patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls: mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client) mock_cls.return_value.__aexit__ = AsyncMock(return_value=None) result = await Service()._determine_api_public_health() @@ -182,7 +183,7 @@ async def test_determine_api_authenticated_health_degraded_response() -> None: mock_client.get.return_value = mock_response with ( - patch("aignostics.platform._service.httpx.AsyncClient") as mock_cls, + patch(_PATCH_HTTPX_ASYNC_CLIENT) as mock_cls, patch(_PATCH_AUTH_GETTER, return_value="test-token"), ): mock_cls.return_value.__aenter__ = AsyncMock(return_value=mock_client)