From ebdf39c2daba041bca2f700178714521b714c197 Mon Sep 17 00:00:00 2001 From: malmans2 Date: Tue, 8 Oct 2024 18:04:33 +0200 Subject: [PATCH 1/2] improve legacy logging --- cads_api_client/api_client.py | 4 ++- cads_api_client/catalogue.py | 4 ++- cads_api_client/legacy_api_client.py | 25 +++----------- cads_api_client/processing.py | 34 +++++++++++++++---- cads_api_client/profile.py | 4 ++- .../integration_test_70_legacy_api_client.py | 34 ++++++++++++++----- tests/test_10_processing.py | 1 + 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/cads_api_client/api_client.py b/cads_api_client/api_client.py index 4710937..f35a6c3 100644 --- a/cads_api_client/api_client.py +++ b/cads_api_client/api_client.py @@ -2,7 +2,7 @@ import functools import warnings -from typing import Any, Literal +from typing import Any, Callable, Literal import attrs import multiurl.base @@ -51,6 +51,7 @@ class ApiClient: retry_after: float = 120 maximum_tries: int = 500 session: requests.Session = attrs.field(factory=requests.Session) + _log_callback: Callable[..., None] | None = None def __attrs_post_init__(self) -> None: if self.url is None: @@ -108,6 +109,7 @@ def _get_request_kwargs( download_options=self._download_options, sleep_max=self.sleep_max, cleanup=self.cleanup, + log_callback=self._log_callback, ) @functools.cached_property diff --git a/cads_api_client/catalogue.py b/cads_api_client/catalogue.py index 6b82c54..13c816d 100644 --- a/cads_api_client/catalogue.py +++ b/cads_api_client/catalogue.py @@ -1,7 +1,7 @@ from __future__ import annotations import datetime -from typing import Any +from typing import Any, Callable import attrs import requests @@ -112,6 +112,7 @@ class Catalogue: download_options: dict[str, Any] sleep_max: float cleanup: bool + log_callback: Callable[..., None] | None force_exact_url: bool = False def __attrs_post_init__(self) -> None: @@ -128,6 +129,7 @@ def _request_kwargs(self) -> RequestKwargs: download_options=self.download_options, sleep_max=self.sleep_max, cleanup=self.cleanup, + log_callback=self.log_callback, ) def get_collections(self, **params: Any) -> Collections: diff --git a/cads_api_client/legacy_api_client.py b/cads_api_client/legacy_api_client.py index 6915043..4d453f4 100644 --- a/cads_api_client/legacy_api_client.py +++ b/cads_api_client/legacy_api_client.py @@ -1,19 +1,17 @@ from __future__ import annotations import collections -import functools import logging import typing import warnings from types import TracebackType -from typing import Any, Callable, TypeVar, cast, overload +from typing import Any, Callable, TypeVar, overload import cdsapi.api import multiurl import requests from . import __version__ as cads_api_client_version -from . import processing from .api_client import ApiClient from .processing import Remote, Results @@ -103,7 +101,7 @@ def __init__( self.debug_callback = debug_callback self.session = requests.Session() if session is None else session - self.client = self.logging_decorator(ApiClient)( + self.client = ApiClient( url=self.url, key=self.key, verify=self.verify, @@ -114,6 +112,7 @@ def __init__( retry_after=self.sleep_max, timeout=self.timeout, progress=self.progress, + log_callback=self.log, ) self.debug( "CDSAPI %s", @@ -137,16 +136,6 @@ def raise_not_implemented_error(self) -> None: "This is a beta version. This functionality has not been implemented yet." ) - def logging_decorator(self, func: F) -> F: - @functools.wraps(func) - def wrapper(*args: Any, **kwargs: Any) -> Any: - with LoggingContext( - logger=processing.LOGGER, quiet=self.quiet, debug=self._debug - ): - return func(*args, **kwargs) - - return cast(F, wrapper) - @overload def retrieve(self, name: str, request: dict[str, Any], target: str) -> str: ... @@ -160,20 +149,16 @@ def retrieve( ) -> str | Remote | Results: submitted: Remote | Results if self.wait_until_complete: - submitted = self.logging_decorator(self.client.submit_and_wait_on_results)( + submitted = self.client.submit_and_wait_on_results( collection_id=name, **request, ) else: - submitted = self.logging_decorator(self.client.submit)( + submitted = self.client.submit( collection_id=name, **request, ) - # Decorate legacy methods - submitted.download = self.logging_decorator(submitted.download) # type: ignore[method-assign] - submitted.log = self.log # type: ignore[method-assign] - return submitted if target is None else submitted.download(target) def log(self, level: int, *args: Any, **kwargs: Any) -> None: diff --git a/cads_api_client/processing.py b/cads_api_client/processing.py index 5c04e25..7955312 100644 --- a/cads_api_client/processing.py +++ b/cads_api_client/processing.py @@ -7,7 +7,7 @@ import time import urllib.parse import warnings -from typing import Any, Type, TypedDict, TypeVar +from typing import Any, Callable, Type, TypedDict, TypeVar try: from typing import Self @@ -46,6 +46,7 @@ class RequestKwargs(TypedDict): download_options: dict[str, Any] sleep_max: float cleanup: bool + log_callback: Callable[..., None] | None class ProcessingFailedError(RuntimeError): @@ -96,6 +97,13 @@ def get_level_and_message(message: str) -> tuple[int, str]: return level, message +def log(*args: Any, callback: Callable[..., None] | None = None, **kwargs: Any) -> None: + if callback is None: + LOGGER.log(*args, **kwargs) + else: + callback(*args, **kwargs) + + @attrs.define(slots=False) class ApiResponse: response: requests.Response @@ -106,6 +114,7 @@ class ApiResponse: download_options: dict[str, Any] sleep_max: float cleanup: bool + log_callback: Callable[..., None] | None @property def _request_kwargs(self) -> RequestKwargs: @@ -117,6 +126,7 @@ def _request_kwargs(self) -> RequestKwargs: download_options=self.download_options, sleep_max=self.sleep_max, cleanup=self.cleanup, + log_callback=self.log_callback, ) @classmethod @@ -131,6 +141,7 @@ def from_request( download_options: dict[str, Any], sleep_max: float, cleanup: bool, + log_callback: Callable[..., None] | None, log_messages: bool = True, **kwargs: Any, ) -> T_ApiResponse: @@ -139,11 +150,15 @@ def from_request( robust_request = multiurl.robust(session.request, **retry_options) inputs = kwargs.get("json", {}).get("inputs", {}) - LOGGER.debug(f"{method.upper()} {url} {inputs or ''}".strip()) + log( + logging.DEBUG, + f"{method.upper()} {url} {inputs or ''}".strip(), + callback=log_callback, + ) response = robust_request( method, url, headers=headers, **request_options, **kwargs ) - LOGGER.debug(f"REPLY {response.text}") + log(logging.DEBUG, f"REPLY {response.text}", callback=log_callback) cads_raise_for_status(response) @@ -156,6 +171,7 @@ def from_request( download_options=download_options, sleep_max=sleep_max, cleanup=cleanup, + log_callback=log_callback, ) if log_messages: self.log_messages() @@ -223,8 +239,8 @@ def _from_rel_href(self, rel: str) -> Self | None: out = None return out - def log(self, level: int, *args: Any, **kwargs: Any) -> None: - LOGGER.log(level, *args, **kwargs) + def log(self, *args: Any, **kwargs: Any) -> None: + log(*args, callback=self.log_callback, **kwargs) def info(self, *args: Any, **kwargs: Any) -> None: self.log(logging.INFO, *args, **kwargs) @@ -367,6 +383,7 @@ class Remote: download_options: dict[str, Any] sleep_max: float cleanup: bool + log_callback: Callable[..., None] | None def __attrs_post_init__(self) -> None: self.log_start_time = None @@ -382,6 +399,7 @@ def _request_kwargs(self) -> RequestKwargs: download_options=self.download_options, sleep_max=self.sleep_max, cleanup=self.cleanup, + log_callback=self.log_callback, ) def _log_metadata(self, metadata: dict[str, Any]) -> None: @@ -579,8 +597,8 @@ def reply(self) -> dict[str, Any]: reply.setdefault("request_id", self.request_uid) return reply - def log(self, level: int, *args: Any, **kwargs: Any) -> None: - LOGGER.log(level, *args, **kwargs) + def log(self, *args: Any, **kwargs: Any) -> None: + log(*args, callback=self.log_callback, **kwargs) def info(self, *args: Any, **kwargs: Any) -> None: self.log(logging.INFO, *args, **kwargs) @@ -722,6 +740,7 @@ class Processing: download_options: dict[str, Any] sleep_max: float cleanup: bool + log_callback: Callable[..., None] | None force_exact_url: bool = False def __attrs_post_init__(self) -> None: @@ -738,6 +757,7 @@ def _request_kwargs(self) -> RequestKwargs: download_options=self.download_options, sleep_max=self.sleep_max, cleanup=self.cleanup, + log_callback=self.log_callback, ) def get_processes(self, **params: Any) -> Processes: diff --git a/cads_api_client/profile.py b/cads_api_client/profile.py index a7af344..b424280 100644 --- a/cads_api_client/profile.py +++ b/cads_api_client/profile.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Any +from typing import Any, Callable import attrs import requests @@ -18,6 +18,7 @@ class Profile: download_options: dict[str, Any] sleep_max: float cleanup: bool + log_callback: Callable[..., None] | None force_exact_url: bool = False def __attrs_post_init__(self) -> None: @@ -34,6 +35,7 @@ def _request_kwargs(self) -> processing.RequestKwargs: download_options=self.download_options, sleep_max=self.sleep_max, cleanup=self.cleanup, + log_callback=self.log_callback, ) def _get_api_response( diff --git a/tests/integration_test_70_legacy_api_client.py b/tests/integration_test_70_legacy_api_client.py index f66c813..d8355d8 100644 --- a/tests/integration_test_70_legacy_api_client.py +++ b/tests/integration_test_70_legacy_api_client.py @@ -1,6 +1,7 @@ from __future__ import annotations import contextlib +import logging import os import pathlib import time @@ -200,16 +201,31 @@ def test_legacy_api_client_kwargs(api_root_url: str, api_anon_key: str) -> None: def test_legacy_api_client_logging( - caplog: pytest.LogCaptureFixture, legacy_client: LegacyApiClient + caplog: pytest.LogCaptureFixture, + api_root_url: str, + api_anon_key: str, ) -> None: - legacy_client.info("Info message") - legacy_client.warning("Warning message") - legacy_client.error("Error message") - assert caplog.record_tuples == [ - ("cads_api_client.legacy_api_client", 20, "Info message"), - ("cads_api_client.legacy_api_client", 30, "Warning message"), - ("cads_api_client.legacy_api_client", 40, "Error message"), - ] + logger = logging.getLogger("foo") + client = LegacyApiClient( + url=api_root_url, + key=api_anon_key, + info_callback=logger.info, + warning_callback=logger.warning, + error_callback=logger.error, + debug_callback=logger.debug, + ) + caplog.clear() + with caplog.at_level(logging.DEBUG): + client.debug("Debug message") + client.info("Info message") + client.warning("Warning message") + client.error("Error message") + assert caplog.record_tuples == [ + ("foo", 10, "Debug message"), + ("foo", 20, "Info message"), + ("foo", 30, "Warning message"), + ("foo", 40, "Error message"), + ] def test_legacy_api_client_download( diff --git a/tests/test_10_processing.py b/tests/test_10_processing.py index 15dbc62..00be863 100644 --- a/tests/test_10_processing.py +++ b/tests/test_10_processing.py @@ -273,6 +273,7 @@ def cat() -> catalogue.Catalogue: download_options={}, sleep_max=120, cleanup=False, + log_callback=None, ) From 700ac0f65fe44d9388d670afbee508362a74a0b6 Mon Sep 17 00:00:00 2001 From: malmans2 Date: Tue, 8 Oct 2024 18:09:04 +0200 Subject: [PATCH 2/2] cleanup --- cads_api_client/legacy_api_client.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cads_api_client/legacy_api_client.py b/cads_api_client/legacy_api_client.py index 4d453f4..8173102 100644 --- a/cads_api_client/legacy_api_client.py +++ b/cads_api_client/legacy_api_client.py @@ -105,13 +105,13 @@ def __init__( url=self.url, key=self.key, verify=self.verify, - sleep_max=self.sleep_max, - session=self.session, - cleanup=self.delete, - maximum_tries=self.retry_max, - retry_after=self.sleep_max, timeout=self.timeout, progress=self.progress, + cleanup=self.delete, + sleep_max=self.sleep_max, + retry_after=self.sleep_max, + maximum_tries=self.retry_max, + session=self.session, log_callback=self.log, ) self.debug(