STL-2767: Retry on HTTP 429 with Retry-After support

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

packages/gooddata-sdk/src/gooddata_sdk/client.py

@@ -3,18 +3,101 @@
 
 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,
+        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 +111,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 +128,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 +156,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 +175,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 +207,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,

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