More updates with feedback

Author: SamRemis

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

@@ -84,7 +84,7 @@ private void generateService(PythonWriter writer) {
             }
 
             writer.addDependency(SmithyPythonDependency.SMITHY_CORE);
-            writer.addImport("smithy_core.retries", "CachingRetryStrategyResolver");
+            writer.addImport("smithy_core.retries", "RetryStrategyResolver");
             writer.write("""
                     def __init__(self, config: $1T | None = None, plugins: list[$2T] | None = None):
                         self._config = config or $1T()
@@ -98,7 +98,7 @@ def __init__(self, config: $1T | None = None, plugins: list[$2T] | None = None):
                         for plugin in client_plugins:
                             plugin(self._config)
 
-                        self._retry_strategy_resolver = CachingRetryStrategyResolver()
+                        self._retry_strategy_resolver = RetryStrategyResolver()
                     """, configSymbol, pluginSymbol, writer.consumer(w -> writeDefaultPlugins(w, defaultPlugins)));
 
             var topDownIndex = TopDownIndex.of(model);

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

@@ -181,13 +181,13 @@ private void generateRequestTest(OperationShape operation, HttpRequestTestCase t
                     } else {
                         path = "";
                     }
-                    writer.addImport("smithy_core.retries", "RetryStrategyOptions");
+                    writer.addImport("smithy_core.retries", "SimpleRetryStrategy");
                     writeClientBlock(context.symbolProvider().toSymbol(service), testCase, Optional.of(() -> {
                         writer.write("""
                                 config = $T(
                                     endpoint_uri="https://$L/$L",
                                     transport = $T(),
-                                    retry_strategy=RetryStrategyOptions(max_attempts=1),
+                                    retry_strategy=SimpleRetryStrategy(max_attempts=1),
                                 )
                                 """,
                                 CodegenUtils.getConfigSymbol(context.settings()),

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

@@ -1,12 +1,7 @@
 #  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 #  SPDX-License-Identifier: Apache-2.0
-from __future__ import annotations
-
 from dataclasses import dataclass
-from typing import TYPE_CHECKING, Literal, Protocol, runtime_checkable
-
-if TYPE_CHECKING:
-    from smithy_core.retries import RetryStrategyOptions
+from typing import Protocol, runtime_checkable
 
 
 @runtime_checkable
@@ -57,9 +52,6 @@ class RetryToken(Protocol):
     """Delay in seconds to wait before the retry attempt."""
 
 
-RetryStrategyType = Literal["simple"]
-
-
 @runtime_checkable
 class RetryStrategy(Protocol):
     """Issuer of :py:class:`RetryToken`s."""
@@ -109,14 +101,3 @@ def record_success(self, *, token: RetryToken) -> None:
         :param token: The token used for the previous successful attempt.
         """
         ...
-
-
-class RetryStrategyResolver[RS: RetryStrategy](Protocol):
-    """Used to resolve a RetryStrategy from retry options."""
-
-    async def resolve_retry_strategy(self, *, options: RetryStrategyOptions) -> RS:
-        """Resolve the retry strategy from the provided options.
-
-        :param options: The retry strategy options to use for creating the strategy.
-        """
-        ...

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

@@ -1,14 +1,17 @@
 #  Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 #  SPDX-License-Identifier: Apache-2.0
 import random
-import threading
 from collections.abc import Callable
 from dataclasses import dataclass
 from enum import Enum
+from functools import lru_cache
+from typing import Literal
 
 from .exceptions import RetryError
 from .interfaces import retries as retries_interface
-from .interfaces.retries import RetryStrategy, RetryStrategyResolver, RetryStrategyType
+from .interfaces.retries import RetryStrategy
+
+RetryStrategyType = Literal["simple"]
 
 
 @dataclass(kw_only=True, frozen=True)
@@ -22,33 +25,26 @@ class RetryStrategyOptions:
     """Maximum number of attempts (initial attempt plus retries)."""
 
 
-class CachingRetryStrategyResolver(RetryStrategyResolver[RetryStrategy]):
+class RetryStrategyResolver:
     """Retry strategy resolver that caches retry strategies based on configuration options.
 
     This resolver caches retry strategy instances based on their configuration to reuse existing
-    instances of RetryStrategy with the same settings.
+    instances of RetryStrategy with the same settings. Uses LRU cache for thread-safe caching.
     """
 
-    def __init__(self) -> None:
-        self._cache: dict[RetryStrategyOptions, RetryStrategy] = {}
-        self._lock = threading.Lock()
-
     async def resolve_retry_strategy(
         self, *, options: RetryStrategyOptions
     ) -> RetryStrategy:
         """Resolve a retry strategy from the provided options, using cache when possible.
 
         :param options: The retry strategy options to use for creating the strategy.
         """
-        with self._lock:
-            if options not in self._cache:
-                self._cache[options] = self._create_retry_strategy(
-                    options.retry_mode, options.max_attempts
-                )
-            return self._cache[options]
+        return self._create_retry_strategy(options.retry_mode, options.max_attempts)
 
+    @staticmethod
+    @lru_cache
     def _create_retry_strategy(
-        self, retry_mode: RetryStrategyType, max_attempts: int
+        retry_mode: RetryStrategyType, max_attempts: int
     ) -> RetryStrategy:
         match retry_mode:
             case "simple":

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

@@ -3,7 +3,7 @@
 import pytest
 from smithy_core.exceptions import CallError, RetryError
 from smithy_core.retries import (
-    CachingRetryStrategyResolver,
+    RetryStrategyResolver,
     ExponentialRetryBackoffStrategy,
     RetryStrategyOptions,
     SimpleRetryStrategy,
@@ -109,7 +109,7 @@ def test_simple_retry_does_not_retry_unsafe() -> None:
 
 
 async def test_caching_retry_strategy_default_resolution() -> None:
-    resolver = CachingRetryStrategyResolver()
+    resolver = RetryStrategyResolver()
     options = RetryStrategyOptions()
 
     strategy = await resolver.resolve_retry_strategy(options=options)
@@ -119,7 +119,7 @@ async def test_caching_retry_strategy_default_resolution() -> None:
 
 
 async def test_caching_retry_strategy_resolver_creates_strategies_by_options() -> None:
-    resolver = CachingRetryStrategyResolver()
+    resolver = RetryStrategyResolver()
 
     options1 = RetryStrategyOptions(max_attempts=3)
     options2 = RetryStrategyOptions(max_attempts=5)
@@ -132,7 +132,7 @@ async def test_caching_retry_strategy_resolver_creates_strategies_by_options() -
 
 
 async def test_caching_retry_strategy_resolver_caches_strategies() -> None:
-    resolver = CachingRetryStrategyResolver()
+    resolver = RetryStrategyResolver()
 
     options = RetryStrategyOptions(max_attempts=5)
     strategy1 = await resolver.resolve_retry_strategy(options=options)