-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Update documentation for default retry strategy change #9622
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
base: develop
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #9622 +/- ##
===========================================
- Coverage 93.40% 93.40% -0.01%
===========================================
Files 211 211
Lines 17012 17041 +29
===========================================
+ Hits 15890 15917 +27
- Misses 1122 1124 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Could we include a changelog entry too? Even though the change is from botocore, I think it's important enough to call out. |
@ashovlin That's actually part of the release process. Everything from botocore gets propogated up in these commits to the CLI. We used to specifically tag them as botocore changes, but it looks like the current release system stopped doing that in ~1.23.0. Maybe a question for @hssyoo if that wasn't intentional. The botocore release will push up both of these: |
Does that include changes that aren't Botocore 1.39.15 had the SSO fix: https://github.com/boto/botocore/blob/d71964b8cb377e358f3dd36f30d9a55880d1ae18/CHANGELOG.rst#L21 But I don't see that in the corresponding CLI 1.41.15: https://github.com/aws/aws-cli/blob/4b050a30d0246546da811bd2408e463bfd889e36/CHANGELOG.rst#L15-L20 EDIT: Clarified internally, this one is a |
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.
Even though it's mentioned under max_attempts
, can we also include the number of attempts it makes in legacy
and standard
retry modes? Something along the lines of 'This retry mode defaults to 4 retries (5 max attempts).' This would be helpful to our users.
@ubaskota I would be more inclined to link to our user guide that details the behavior, specifics, and caveats of each strategy: https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-retries.html#cli-usage-retries-modes What do you think of that? |
Also add a link to the CLI user guide that gives full details on the different retry strategies.
Addressed in 93321e0. |
Yeah, that works too. |
Issue #, if available:
boto/botocore#3513
Description of changes:
Updates the configuration options topic guide for retries to indicate the default is changing from
legacy
tostandard
.The AWS CLI user guide will be updated separately to reflect the change as well.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.