-
Notifications
You must be signed in to change notification settings - Fork 30
Refactor RetryStrategy to be async #671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
JordonPhillips
wants to merge
2
commits into
develop
Choose a base branch
from
async-retry-2
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4 changes: 4 additions & 0 deletions
4
...thy-core/.changes/next-release/smithy-core-breaking-e1d59c61d8344b88832fc481dbc16531.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| { | ||
| "type": "breaking", | ||
| "description": "Refactored retry strategies to be async, allowing them to wait internally or use async synchronization primitives if necessary." | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
56 changes: 56 additions & 0 deletions
56
packages/smithy-core/src/smithy_core/aio/interfaces/retries.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| from typing import Protocol, runtime_checkable | ||
|
|
||
| from ...interfaces.retries import RetryBackoffStrategy, RetryToken | ||
|
|
||
|
|
||
| @runtime_checkable | ||
| class RetryStrategy(Protocol): | ||
| """Issuer of :py:class:`RetryToken`s.""" | ||
|
|
||
| backoff_strategy: RetryBackoffStrategy | ||
| """The strategy used by returned tokens to compute delay duration values.""" | ||
|
|
||
| max_attempts: int | ||
| """Upper limit on total attempt count (initial attempt plus retries).""" | ||
|
|
||
| async def acquire_initial_retry_token( | ||
| self, *, token_scope: str | None = None | ||
| ) -> RetryToken: | ||
| """Create a base retry token for the start of a request. | ||
|
|
||
| :param token_scope: An arbitrary string accepted by the retry strategy to | ||
| separate tokens into scopes. | ||
| :returns: A retry token, to be used for determining the retry delay, refreshing | ||
| the token after a failure, and recording success after success. | ||
| :raises RetryError: If the retry strategy has no available tokens. | ||
| """ | ||
| ... | ||
|
|
||
| async def refresh_retry_token_for_retry( | ||
| self, *, token_to_renew: RetryToken, error: Exception | ||
| ) -> RetryToken: | ||
| """Replace an existing retry token from a failed attempt with a new token. | ||
|
|
||
| After a failed operation call, this method is called to exchange a retry token | ||
| that was previously obtained by calling :py:func:`acquire_initial_retry_token` | ||
| or this method with a new retry token for the next attempt. This method can | ||
| either choose to allow another retry and send a new or updated token, or reject | ||
| the retry attempt and raise the error. | ||
|
|
||
| :param token_to_renew: The token used for the previous failed attempt. | ||
| :param error: The error that triggered the need for a retry. | ||
| :raises RetryError: If no further retry attempts are allowed. | ||
| """ | ||
| ... | ||
|
|
||
| async def record_success(self, *, token: RetryToken) -> None: | ||
| """Return token after successful completion of an operation. | ||
|
|
||
| Upon successful completion of the operation, a user calls this function to | ||
| record that the operation was successful. | ||
|
|
||
| :param token: The token used for the previous successful attempt. | ||
| """ | ||
| ... |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - Looks like there are at least two additional lines missing
await. The othertransport_client.sendbelow needs this too. Not sure if deserialize also needs one. Our real code insmithy_coreawaits deserialization.