Skip to content

Commit 12af0f3

Browse files
authored
Implement Retry Strategy Resolver Pattern with Per-Client Caching (#600)
1 parent 84fc260 commit 12af0f3

File tree

5 files changed

+130
-13
lines changed

5 files changed

+130
-13
lines changed

codegen/core/src/main/java/software/amazon/smithy/python/codegen/ClientGenerator.java

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ private void generateService(PythonWriter writer) {
8383
}
8484
}
8585

86+
writer.addDependency(SmithyPythonDependency.SMITHY_CORE);
87+
writer.addImport("smithy_core.retries", "RetryStrategyResolver");
8688
writer.write("""
8789
def __init__(self, config: $1T | None = None, plugins: list[$2T] | None = None):
8890
self._config = config or $1T()
@@ -95,6 +97,8 @@ def __init__(self, config: $1T | None = None, plugins: list[$2T] | None = None):
9597
9698
for plugin in client_plugins:
9799
plugin(self._config)
100+
101+
self._retry_strategy_resolver = RetryStrategyResolver()
98102
""", configSymbol, pluginSymbol, writer.consumer(w -> writeDefaultPlugins(w, defaultPlugins)));
99103

100104
var topDownIndex = TopDownIndex.of(model);
@@ -187,6 +191,8 @@ private void writeSharedOperationInit(PythonWriter writer, OperationShape operat
187191
writer.addImport("smithy_core.types", "TypedProperties");
188192
writer.addImport("smithy_core.aio.client", "RequestPipeline");
189193
writer.addImport("smithy_core.exceptions", "ExpectationNotMetError");
194+
writer.addImport("smithy_core.retries", "RetryStrategyOptions");
195+
writer.addImport("smithy_core.interfaces.retries", "RetryStrategy");
190196
writer.addStdlibImport("copy", "deepcopy");
191197

192198
writer.write("""
@@ -200,6 +206,24 @@ private void writeSharedOperationInit(PythonWriter writer, OperationShape operat
200206
plugin(config)
201207
if config.protocol is None or config.transport is None:
202208
raise ExpectationNotMetError("protocol and transport MUST be set on the config to make calls.")
209+
210+
# Resolve retry strategy from config
211+
if isinstance(config.retry_strategy, RetryStrategy):
212+
retry_strategy = config.retry_strategy
213+
elif isinstance(config.retry_strategy, RetryStrategyOptions):
214+
retry_strategy = await self._retry_strategy_resolver.resolve_retry_strategy(
215+
options=config.retry_strategy
216+
)
217+
elif config.retry_strategy is None:
218+
retry_strategy = await self._retry_strategy_resolver.resolve_retry_strategy(
219+
options=RetryStrategyOptions()
220+
)
221+
else:
222+
raise TypeError(
223+
f"retry_strategy must be RetryStrategy, RetryStrategyOptions, or None, "
224+
f"got {type(config.retry_strategy).__name__}"
225+
)
226+
203227
pipeline = RequestPipeline(
204228
protocol=config.protocol,
205229
transport=config.transport
@@ -212,7 +236,7 @@ raise ExpectationNotMetError("protocol and transport MUST be set on the config t
212236
auth_scheme_resolver=config.auth_scheme_resolver,
213237
supported_auth_schemes=config.auth_schemes,
214238
endpoint_resolver=config.endpoint_resolver,
215-
retry_strategy=config.retry_strategy,
239+
retry_strategy=retry_strategy,
216240
)
217241
""", writer.consumer(w -> writeDefaultPlugins(w, defaultPlugins)));
218242

codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,20 @@ public final class ConfigGenerator implements Runnable {
5555
ConfigProperty.builder()
5656
.name("retry_strategy")
5757
.type(Symbol.builder()
58-
.name("RetryStrategy")
59-
.namespace("smithy_core.interfaces.retries", ".")
60-
.addDependency(SmithyPythonDependency.SMITHY_CORE)
58+
.name("RetryStrategy | RetryStrategyOptions")
59+
.addReference(Symbol.builder()
60+
.name("RetryStrategy")
61+
.namespace("smithy_core.interfaces.retries", ".")
62+
.addDependency(SmithyPythonDependency.SMITHY_CORE)
63+
.build())
64+
.addReference(Symbol.builder()
65+
.name("RetryStrategyOptions")
66+
.namespace("smithy_core.retries", ".")
67+
.addDependency(SmithyPythonDependency.SMITHY_CORE)
68+
.build())
6169
.build())
62-
.documentation("The retry strategy for issuing retry tokens and computing retry delays.")
63-
.nullable(false)
64-
.initialize(writer -> {
65-
writer.addDependency(SmithyPythonDependency.SMITHY_CORE);
66-
writer.addImport("smithy_core.retries", "SimpleRetryStrategy");
67-
writer.write("self.retry_strategy = retry_strategy or SimpleRetryStrategy()");
68-
})
70+
.documentation(
71+
"The retry strategy or options for configuring retry behavior. Can be either a configured RetryStrategy or RetryStrategyOptions to create one.")
6972
.build(),
7073
ConfigProperty.builder()
7174
.name("endpoint_uri")
@@ -379,7 +382,7 @@ private void writeInitParams(PythonWriter writer, Collection<ConfigProperty> pro
379382
}
380383

381384
private void documentProperties(PythonWriter writer, Collection<ConfigProperty> properties) {
382-
writer.writeDocs(() ->{
385+
writer.writeDocs(() -> {
383386
var iter = properties.iterator();
384387
writer.write("\nConstructor.\n");
385388
while (iter.hasNext()) {

packages/smithy-core/src/smithy_core/interfaces/retries.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ class RetryToken(Protocol):
5555
"""Delay in seconds to wait before the retry attempt."""
5656

5757

58+
@runtime_checkable
5859
class RetryStrategy(Protocol):
5960
"""Issuer of :py:class:`RetryToken`s."""
6061

packages/smithy-core/src/smithy_core/retries.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,58 @@
55
from collections.abc import Callable
66
from dataclasses import dataclass
77
from enum import Enum
8+
from functools import lru_cache
9+
from typing import Any, Literal
810

911
from .exceptions import RetryError
1012
from .interfaces import retries as retries_interface
13+
from .interfaces.retries import RetryStrategy
14+
15+
RetryStrategyType = Literal["simple", "standard"]
16+
17+
18+
@dataclass(kw_only=True, frozen=True)
19+
class RetryStrategyOptions:
20+
"""Options for configuring retry behavior."""
21+
22+
retry_mode: RetryStrategyType = "standard"
23+
"""The retry mode to use."""
24+
25+
max_attempts: int | None = None
26+
"""Maximum number of attempts (initial attempt plus retries). If None, uses the strategy's default."""
27+
28+
29+
class RetryStrategyResolver:
30+
"""Retry strategy resolver that caches retry strategies based on configuration options.
31+
32+
This resolver caches retry strategy instances based on their configuration to reuse existing
33+
instances of RetryStrategy with the same settings. Uses LRU cache for thread-safe caching.
34+
"""
35+
36+
async def resolve_retry_strategy(
37+
self, *, options: RetryStrategyOptions
38+
) -> RetryStrategy:
39+
"""Resolve a retry strategy from the provided options, using cache when possible.
40+
41+
:param options: The retry strategy options to use for creating the strategy.
42+
"""
43+
return self._create_retry_strategy(options.retry_mode, options.max_attempts)
44+
45+
@lru_cache
46+
def _create_retry_strategy(
47+
self, retry_mode: RetryStrategyType, max_attempts: int | None
48+
) -> RetryStrategy:
49+
kwargs = {"max_attempts": max_attempts}
50+
filtered_kwargs: dict[str, Any] = {
51+
k: v for k, v in kwargs.items() if v is not None
52+
}
53+
match retry_mode:
54+
case "simple":
55+
return SimpleRetryStrategy(**filtered_kwargs)
56+
case "standard":
57+
return StandardRetryStrategy(**filtered_kwargs)
58+
case _:
59+
raise ValueError(f"Unknown retry mode: {retry_mode}")
1160

1261

1362
class ExponentialBackoffJitterType(Enum):
@@ -244,6 +293,9 @@ def refresh_retry_token_for_retry(
244293
def record_success(self, *, token: retries_interface.RetryToken) -> None:
245294
"""Not used by this retry strategy."""
246295

296+
def __deepcopy__(self, memo: Any) -> "SimpleRetryStrategy":
297+
return self
298+
247299

248300
class StandardRetryQuota:
249301
"""Retry quota used by :py:class:`StandardRetryStrategy`."""
@@ -414,3 +466,6 @@ def record_success(self, *, token: retries_interface.RetryToken) -> None:
414466
f"StandardRetryStrategy requires StandardRetryToken, got {type(token).__name__}"
415467
)
416468
self._retry_quota.release(release_amount=token.quota_acquired)
469+
470+
def __deepcopy__(self, memo: Any) -> "StandardRetryStrategy":
471+
return self

packages/smithy-core/tests/unit/test_retries.py

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
# SPDX-License-Identifier: Apache-2.0
3-
43
import pytest
54
from smithy_core.exceptions import CallError, RetryError
65
from smithy_core.retries import ExponentialBackoffJitterType as EBJT
76
from smithy_core.retries import (
87
ExponentialRetryBackoffStrategy,
8+
RetryStrategyOptions,
9+
RetryStrategyResolver,
910
SimpleRetryStrategy,
1011
StandardRetryQuota,
1112
StandardRetryStrategy,
@@ -217,3 +218,36 @@ def test_retry_quota_acquire_timeout_error(
217218
acquired = retry_quota.acquire(error=timeout_error)
218219
assert acquired == StandardRetryQuota.TIMEOUT_RETRY_COST
219220
assert retry_quota.available_capacity == 0
221+
222+
223+
async def test_caching_retry_strategy_default_resolution() -> None:
224+
resolver = RetryStrategyResolver()
225+
options = RetryStrategyOptions()
226+
227+
strategy = await resolver.resolve_retry_strategy(options=options)
228+
229+
assert isinstance(strategy, StandardRetryStrategy)
230+
assert strategy.max_attempts == 3
231+
232+
233+
async def test_caching_retry_strategy_resolver_creates_strategies_by_options() -> None:
234+
resolver = RetryStrategyResolver()
235+
236+
options1 = RetryStrategyOptions(max_attempts=3)
237+
options2 = RetryStrategyOptions(max_attempts=5)
238+
239+
strategy1 = await resolver.resolve_retry_strategy(options=options1)
240+
strategy2 = await resolver.resolve_retry_strategy(options=options2)
241+
242+
assert strategy1.max_attempts == 3
243+
assert strategy2.max_attempts == 5
244+
245+
246+
async def test_caching_retry_strategy_resolver_caches_strategies() -> None:
247+
resolver = RetryStrategyResolver()
248+
249+
options = RetryStrategyOptions(max_attempts=5)
250+
strategy1 = await resolver.resolve_retry_strategy(options=options)
251+
strategy2 = await resolver.resolve_retry_strategy(options=options)
252+
253+
assert strategy1 is strategy2

0 commit comments

Comments
 (0)