From 9012fae4cce7e9d12d81fe18437072982e28f280 Mon Sep 17 00:00:00 2001 From: Roman Kiselevich Date: Tue, 26 May 2026 16:35:29 +0200 Subject: [PATCH] feat(gooddata-sdk): retry on HTTP 429 with Retry-After support Adds GoodDataApiClientRetryConfig, applied to both the generated api-client (via Configuration.retries) and the direct requests.post in _do_post_request (via a Session with HTTPAdapter). Defaults: 10 retries on 429, backoff_factor=0.5, backoff_max=60s; Retry-After honoured automatically. jira: STL-2767 risk: low --- .../gooddata-sdk/src/gooddata_sdk/__init__.py | 2 +- .../gooddata-sdk/src/gooddata_sdk/client.py | 103 +++++++++++++++++- packages/gooddata-sdk/tests/test_client.py | 56 +++++++++- 3 files changed, 158 insertions(+), 3 deletions(-) diff --git a/packages/gooddata-sdk/src/gooddata_sdk/__init__.py b/packages/gooddata-sdk/src/gooddata_sdk/__init__.py index 91f87c918..a23d22e66 100644 --- a/packages/gooddata-sdk/src/gooddata_sdk/__init__.py +++ b/packages/gooddata-sdk/src/gooddata_sdk/__init__.py @@ -280,7 +280,7 @@ CatalogUserDataFilterRelationships, ) from gooddata_sdk.catalog.workspace.entity_model.workspace import CatalogWorkspace -from gooddata_sdk.client import GoodDataApiClient +from gooddata_sdk.client import GoodDataApiClient, GoodDataApiClientRetryConfig from gooddata_sdk.compute.compute_to_sdk_converter import ComputeToSdkConverter from gooddata_sdk.compute.model.attribute import Attribute from gooddata_sdk.compute.model.base import ExecModelEntity, ObjId diff --git a/packages/gooddata-sdk/src/gooddata_sdk/client.py b/packages/gooddata-sdk/src/gooddata_sdk/client.py index 0fd65a276..5af707cfc 100644 --- a/packages/gooddata-sdk/src/gooddata_sdk/client.py +++ b/packages/gooddata-sdk/src/gooddata_sdk/client.py @@ -3,18 +3,105 @@ from __future__ import annotations +import logging import os +from dataclasses import dataclass, field from pathlib import Path import gooddata_api_client as api_client import requests from gooddata_api_client import apis +from requests.adapters import HTTPAdapter +from urllib3.exceptions import MaxRetryError +from urllib3.util.retry import Retry from gooddata_sdk import __version__ from gooddata_sdk.utils import HttpMethod +logger = logging.getLogger(__name__) + USER_AGENT = f"gooddata-python-sdk/{__version__}" +DEFAULT_RETRY_ALLOWED_METHODS: frozenset[str] = frozenset( + ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE", "POST", "PATCH"] +) + + +@dataclass(frozen=True) +class GoodDataApiClientRetryConfig: + """Retry policy for transient HTTP failures. + + The same policy is applied to both transport paths: + - the generated `gooddata-api-client` (via `urllib3` `Retry`) + - the direct `requests`-based POST in `GoodDataApiClient._do_post_request` + (via `HTTPAdapter` mounted on a `Session`) + + `Retry-After` from the server is honoured automatically; `backoff_factor` + only applies when that header is absent. + """ + + max_retries: int = 10 + backoff_factor: float = 0.5 + backoff_max: float = 60.0 + status_forcelist: tuple[int, ...] = (429,) + allowed_methods: frozenset[str] = field(default_factory=lambda: DEFAULT_RETRY_ALLOWED_METHODS) + + +class _LoggingRetry(Retry): + """Retry that logs each rate-limit hit and final exhaustion. + + Logs at WARNING when a configured status (HTTP 429 by default) is + received and a retry is scheduled, and at ERROR when retries are + exhausted. Other retry causes (connection errors, redirects, etc.) + are left to urllib3's own logging. + """ + + def increment( # type: ignore[override] + self, + method=None, + url=None, + response=None, + error=None, + _pool=None, + _stacktrace=None, + ): + if response is not None and response.status in self.status_forcelist: + logger.warning( + "GoodData API rate-limited: %s %s -> %s; Retry-After=%s; retries left=%s", + method, + url, + response.status, + response.headers.get("Retry-After"), + self.total, + ) + try: + return super().increment(method, url, response, error, _pool, _stacktrace) + except MaxRetryError: + logger.error( + "GoodData API rate-limit retries exhausted: %s %s -> %s", + method, + url, + response.status, + ) + raise + return super().increment(method, url, response, error, _pool, _stacktrace) + + +def _build_urllib3_retry(retry_config: GoodDataApiClientRetryConfig) -> Retry: + return _LoggingRetry( + total=retry_config.max_retries, + connect=0, + read=0, + other=0, + status=retry_config.max_retries, + backoff_factor=retry_config.backoff_factor, + backoff_max=retry_config.backoff_max, + status_forcelist=retry_config.status_forcelist, + allowed_methods=retry_config.allowed_methods, + respect_retry_after_header=True, + raise_on_status=False, + ) + class GoodDataApiClient: """Provide access to metadata and afm services.""" @@ -28,6 +115,7 @@ def __init__( executions_cancellable: bool = False, ssl_ca_cert: str | None = None, proxy: str | None = None, + retry_config: GoodDataApiClientRetryConfig | None = None, ) -> None: """Take url, token for connecting to GoodData.CN. @@ -44,6 +132,10 @@ def __init__( `proxy` is optional URL of an HTTP(S) proxy (e.g. ``http://proxy:8080``). When not set, the standard ``HTTPS_PROXY`` / ``https_proxy`` / ``HTTP_PROXY`` / ``http_proxy`` environment variables are checked automatically. + + `retry_config` controls retry behaviour for transient HTTP failures + (HTTP 429 by default). When omitted, sensible defaults are used and + ``Retry-After`` is honoured automatically. """ self._hostname = host self._token = token @@ -68,7 +160,11 @@ def __init__( or None ) + self._retry_config = retry_config or GoodDataApiClientRetryConfig() + self._retry = _build_urllib3_retry(self._retry_config) + self._api_config = api_client.Configuration(host=host, ssl_ca_cert=ssl_ca_cert) + self._api_config.retries = self._retry if proxy: self._api_config.proxy = proxy self._api_client = api_client.ApiClient( @@ -83,6 +179,11 @@ def __init__( self._api_client.default_headers[header_name] = header_value self._api_client.user_agent = user_agent + self._session = requests.Session() + adapter = HTTPAdapter(max_retries=_build_urllib3_retry(self._retry_config)) + self._session.mount("http://", adapter) + self._session.mount("https://", adapter) + self._entities_api = apis.EntitiesApi(self._api_client) self._layout_api = apis.LayoutApi(self._api_client) self._actions_api = apis.ActionsApi(self._api_client) @@ -110,7 +211,7 @@ def _do_post_request( if not self._hostname.endswith("/"): endpoint = f"/{endpoint}" - response = requests.post( + response = self._session.post( url=f"{self._hostname}{endpoint}", headers={ "Content-Type": content_type, diff --git a/packages/gooddata-sdk/tests/test_client.py b/packages/gooddata-sdk/tests/test_client.py index dac94a308..342436665 100644 --- a/packages/gooddata-sdk/tests/test_client.py +++ b/packages/gooddata-sdk/tests/test_client.py @@ -2,7 +2,8 @@ import os from unittest import mock -from gooddata_sdk import GoodDataApiClient, GoodDataSdk +from gooddata_sdk import GoodDataApiClient, GoodDataApiClientRetryConfig, GoodDataSdk +from urllib3.util.retry import Retry def test_http_headers_precedence(): @@ -74,3 +75,56 @@ def test_sdk_create_picks_up_env_proxy(self): def test_sdk_create_no_proxy_when_env_empty(self): sdk = GoodDataSdk.create("https://example.com", "token") assert sdk.client._api_config.proxy is None + + +class TestGoodDataApiClientRetryConfig: + """Retry/back-off config propagates to both transport paths.""" + + def test_defaults_match_public_rate_limit_contract(self): + c = GoodDataApiClient("https://example.com", "token") + retry = c._api_config.retries + assert isinstance(retry, Retry) + assert retry.total == 10 + assert retry.backoff_factor == 0.5 + assert 429 in retry.status_forcelist + assert retry.respect_retry_after_header is True + for method in ("GET", "POST", "PUT", "PATCH", "DELETE"): + assert method in retry.allowed_methods + + def test_custom_retry_config_overrides_defaults(self): + cfg = GoodDataApiClientRetryConfig( + max_retries=3, + backoff_factor=2.0, + status_forcelist=(429, 503), + allowed_methods=frozenset(["GET", "HEAD"]), + ) + c = GoodDataApiClient("https://example.com", "token", retry_config=cfg) + retry = c._api_config.retries + assert retry.total == 3 + assert retry.backoff_factor == 2.0 + assert retry.status_forcelist == (429, 503) + assert retry.allowed_methods == frozenset(["GET", "HEAD"]) + + def test_session_adapter_uses_same_retry_policy(self): + c = GoodDataApiClient("https://example.com", "token") + for scheme in ("http://", "https://"): + adapter = c._session.get_adapter(scheme + "example.com") + assert adapter.max_retries.total == c._retry_config.max_retries + assert adapter.max_retries.status_forcelist == c._retry_config.status_forcelist + + def test_sdk_wraps_client_with_custom_retry_config(self): + cfg = GoodDataApiClientRetryConfig(max_retries=2) + client = GoodDataApiClient("https://example.com", "token", retry_config=cfg) + sdk = GoodDataSdk(client) + assert sdk.client._api_config.retries.total == 2 + + def test_rate_limit_hit_logs_warning(self): + c = GoodDataApiClient("https://example.com", "token") + retry = c._api_config.retries + fake_response = mock.Mock(status=429, headers={"Retry-After": "2"}) + with mock.patch("gooddata_sdk.client.logger") as mock_logger: + retry.increment(method="POST", url="/api/v1/entities/users", response=fake_response) + assert mock_logger.warning.called + fmt = mock_logger.warning.call_args.args[0] + assert "rate-limited" in fmt + assert mock_logger.error.called is False