Skip to content
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

Make the number of retries configurable #595

Merged
merged 5 commits into from
Mar 20, 2023
Merged

Conversation

iamed2
Copy link
Member

@iamed2 iamed2 commented Dec 17, 2022

This adds a max_attempts kwarg+field to AWSConfig, and a max_attempts accessor for AbstractAWSConfig (defaulting to 3, which is the default elsewhere in the code).

I saw most of the tests are done with S3, and I also saw that the S3 throttling error (SlowDown) was not included. I added that error code and then added a test that uses it and tests throttling retries.


test/AWS.jl Outdated Show resolved Hide resolved
test/patch.jl Outdated Show resolved Hide resolved
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@iamed2
Copy link
Member Author

iamed2 commented Dec 17, 2022

I think likely there are a few more throttling error codes we could add in, in a future PR: https://boto3.amazonaws.com/v1/documentation/api/latest/guide/retries.html#standard-retry-mode

@iamed2
Copy link
Member Author

iamed2 commented Dec 17, 2022

bors try

bors bot added a commit that referenced this pull request Dec 17, 2022
@bors
Copy link
Contributor

bors bot commented Dec 17, 2022

@ericphanson
Copy link
Member

AWS has a conventional env variable btw: #551

@@ -190,7 +191,7 @@ function submit_request(aws::AbstractAWSConfig, request::Request; return_headers
return false
end

delays = AWSExponentialBackoff(; max_attempts=3)
delays = AWSExponentialBackoff(; max_attempts=max_attempts(aws))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about making the whole delay strategy configurable?

Suggested change
delays = AWSExponentialBackoff(; max_attempts=max_attempts(aws))
delays = delays(aws)

to give more control, e.g. tweak the initial delay or delay between retries.

Perhaps this would require exporting AWSExponentialBackoff so it's easy for users to tweak only the max attempt.

@mattBrzezinski
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Mar 20, 2023
@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

Copy link
Member

@mattBrzezinski mattBrzezinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 20, 2023

@bors bors bot merged commit f738b28 into master Mar 20, 2023
@bors bors bot deleted the ed/config-max-attempts branch March 20, 2023 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants