Skip to content

Commit 3f3a416

Browse files
committed
Use exponential backoff when missing Retry-After header.
1 parent 6d27f7e commit 3f3a416

File tree

3 files changed

+26
-24
lines changed

3 files changed

+26
-24
lines changed

src/ghga_service_commons/transports/factory.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,15 @@ class CompositeTransportFactory:
2727
"""TODO"""
2828

2929
@classmethod
30-
def _create_common_transport_layers(cls, config: CompositeConfig, transport):
30+
def _create_common_transport_layers(
31+
cls, config: CompositeConfig, limits: Limits | None = None
32+
):
3133
"""TODO"""
34+
base_transport = (
35+
AsyncHTTPTransport(limits=limits) if limits else AsyncHTTPTransport()
36+
)
3237
ratelimiting_transport = AsyncRatelimitingTransport(
33-
config=config, transport=transport
38+
config=config, transport=base_transport
3439
)
3540
retry_transport = AsyncRetryTransport(
3641
config=config, transport=ratelimiting_transport
@@ -42,19 +47,13 @@ def create_ratelimiting_retry_transport(
4247
cls, config: CompositeConfig, limits: Limits | None = None
4348
) -> AsyncRetryTransport:
4449
"""TODO"""
45-
base_transport = (
46-
AsyncHTTPTransport(limits=limits) if limits else AsyncHTTPTransport()
47-
)
48-
return cls._create_common_transport_layers(config, base_transport)
50+
return cls._create_common_transport_layers(config)
4951

5052
@classmethod
5153
def create_ratelimiting_retry_transport_with_cache(
5254
cls, config: CompositeCacheConfig, limits: Limits | None = None
53-
) -> AsyncRetryTransport:
55+
) -> AsyncCacheTransport:
5456
"""TODO"""
55-
base_transport = (
56-
AsyncHTTPTransport(limits=limits) if limits else AsyncHTTPTransport()
57-
)
5857
storage = AsyncInMemoryStorage(ttl=config.cache_ttl)
59-
cache_tranport = AsyncCacheTransport(transport=base_transport, storage=storage)
60-
return cls._create_common_transport_layers(config, cache_tranport)
58+
retry_transport = cls._create_common_transport_layers(config)
59+
return AsyncCacheTransport(transport=retry_transport, storage=storage)

src/ghga_service_commons/transports/ratelimiting.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,21 @@ async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
5353
# Caculate seconds since the last request has been fired and corresponding wait time
5454
time_elapsed = time.monotonic() - self._last_request_time
5555
remaining_wait = max(0, self._wait_time - time_elapsed)
56-
log.info("Configured base wait time: %.3f s", self._wait_time)
57-
log.info(
58-
"Time elapsed since last request:%.3f.\nWaiting for at least %.3f s",
56+
log.debug(
57+
"Time elapsed since last request: %.3f s.\nRemaining wait time: %.3f s.",
5958
time_elapsed,
6059
remaining_wait,
6160
)
6261

6362
# Add jitter to both cases and sleep
6463
if remaining_wait < self._jitter:
65-
time.sleep(random.uniform(remaining_wait, self._jitter)) # noqa: S311
64+
sleep_for = random.uniform(remaining_wait, self._jitter) # noqa: S311
65+
log.debug("Sleeping for %.3f s.")
66+
time.sleep(sleep_for)
6667
else:
67-
time.sleep(
68-
random.uniform(remaining_wait, remaining_wait + self._jitter) # noqa: S311
69-
)
68+
sleep_for = random.uniform(remaining_wait, remaining_wait + self._jitter) # noqa: S311
69+
log.debug("Sleeping for %.3f s.")
70+
time.sleep(sleep_for)
7071

7172
# Delegate call and update timestamp
7273
response = await self._transport.handle_async_request(request=request)
@@ -79,12 +80,13 @@ async def handle_async_request(self, request: httpx.Request) -> httpx.Response:
7980
retry_after = response.headers.get("Retry-After")
8081
if retry_after:
8182
self._wait_time = float(retry_after)
82-
log.info("Received retry after response: %.3f s", self._wait_time)
83+
log.info("Received retry after response: %.3f s.", self._wait_time)
8384
else:
8485
log.warning(
85-
"Retry-After header not present in 429 response, using fallback instead."
86+
"Retry-After header not present in 429 response.\nDelegating to underlying wait strategy."
8687
)
87-
self._wait_time = self._jitter
88+
# Modify response headers to communicate intent to retry layer
89+
response.headers["Should-Wait"] = "true"
8890
self._num_requests = 0
8991
elif self._reset_after and self._reset_after <= self._num_requests:
9092
self._wait_time = 0

src/ghga_service_commons/transports/retry.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ def _log_retry_stats(retry_state: RetryCallState):
6363
if time_passed := retry_state.seconds_since_start:
6464
retry_stats["seconds_elapsed"] = round(time_passed, 3)
6565

66-
log.warning(retry_state.fn)
6766
log.info(
6867
"Retry attempt number %i for function %s.",
6968
attempt_number,
@@ -75,11 +74,13 @@ def _log_retry_stats(retry_state: RetryCallState):
7574
class wait_exponential_ignore_429(wait_exponential): # noqa: N801
7675
"""Custom exponential backof strategy not waiting on 429 responses"""
7776

78-
def __call__(self, retry_state: RetryCallState) -> float: # noqa: D102
77+
def __call__(self, retry_state: RetryCallState) -> float:
78+
"""Copied from base class and adjusted"""
7979
if (
8080
retry_state.outcome
8181
and (result := retry_state.outcome.result())
8282
and isinstance(result, httpx.Response)
83+
and not result.headers.get("Should-Wait")
8384
):
8485
return 0
8586
try:

0 commit comments

Comments
 (0)