Implement Retry Strategy Resolver Pattern with Per-Client Caching #600
Conversation
...gen/core/src/main/java/software/amazon/smithy/python/codegen/generators/ConfigGenerator.java
Outdated
Show resolved
Hide resolved
nateprewitt
left a comment
nateprewitt
left a comment
There was a problem hiding this comment.
Not sure what happened with the commits for the last change but once that's fixed this should be set.
SamRemis
force-pushed
the
retry-mode-resolver
branch
from
36749b8 to
a4c632b
Compare
SamRemis
force-pushed
the
retry-mode-resolver
branch
from
a4c632b to
ad9074d
Compare
| case "simple": | ||
| if max_attempts is None: | ||
| return SimpleRetryStrategy() | ||
| else: | ||
| return SimpleRetryStrategy(max_attempts=max_attempts) | ||
| case "standard": | ||
| if max_attempts is None: | ||
| return StandardRetryStrategy() | ||
| else: | ||
| return StandardRetryStrategy(max_attempts=max_attempts) |
There was a problem hiding this comment.
This seems a bit awkward and easy to miss when adding new ones. Can we create a single kwargs list and pass it in to avoid the conditional swap for every method?
There was a problem hiding this comment.
Agreed. Before I added the typehint to the kwargs variable, pyright raised errors like this:
Argument of type "int" cannot be assigned to parameter "backoff_strategy" of type "RetryBackoffStrategy | None" in function "__init__"
I didn't realize you could resolve them by adding a type hint to the kwargs variable. This seems to be an odd behavior of pyright - taking an untyped dictionary that is being unpacked and assuming that the keys might be passed to any given input parameter seems odd.
- Add retry mode resolver functionality with caching - Change default retry mode from simple to standard - Add deepcopy support to retry strategies - Update test coverage for new features # Conflicts: # packages/smithy-core/tests/unit/test_retries.py
SamRemis
force-pushed
the
retry-mode-resolver
branch
from
66eb7aa to
f88f07f
Compare
Co-authored-by: Nate Prewitt <nate.prewitt@gmail.com>
2 participants
Summary
Introduces a resolver pattern for retry strategies that enables per-client caching and state management. This allows multiple operations from the same client to share a single retry strategy instance, which is important for strategies that maintain state across retries such as token buckets and rate limiters.
Motivation
Previously, each operation created its own retry strategy instance, preventing state sharing across operations from the same client. This change implements a resolver-based approach.
Key Changes
RetryStrategyResolverprotocol similar toAuthSchemeResolver. This will be used for objects which will supply clients with retry strategies.CachingRetryStrategyResolverwhich implements the protocol and returns a unique instance of a requested retry strategy object per unique combination of max attempts setting and retry mode setting. This will get called by the client when no RetryStrategy is specified.RetryStrategyOptionsfor users to configure their retry strategy mode and max attempts per client call.