Skip to content

feat: read maxAttempts value from retry-config #1286

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

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

trivikr
Copy link
Member

@trivikr trivikr commented Jun 19, 2020

Issue #, if available:
Follow-up to #1284

Description of changes:

read maxAttempts value from retry-config

  • maxAttempts is now updated from number to Provider<string>
    • Reason: The values in environment variables and config are stored as strings, and retry-config-provider utility functions are shared between maxAttempts (string) and retryMode (string)
  • maxAttempts is removed from RetryStrategy interface
  • retryMiddleware is always added in getRetryPlugin, as maxAttempts value is not available.
    • retryMiddleware decides to not retry in case maxAttempts is less than or equal to 1

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2020

Codecov Report

Merging #1286 into master will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1286      +/-   ##
==========================================
+ Coverage   73.04%   73.17%   +0.13%     
==========================================
  Files         287      290       +3     
  Lines       12730    12792      +62     
  Branches     2922     2936      +14     
==========================================
+ Hits         9299     9361      +62     
  Misses       3431     3431              
Impacted Files Coverage Δ
clients/client-s3/S3Client.ts 94.11% <0.00%> (ø)
clients/client-s3/runtimeConfig.ts 100.00% <0.00%> (ø)
packages/region-provider/src/fromEnv.ts 100.00% <0.00%> (ø)
protocol_tests/aws-ec2/runtimeConfig.ts 100.00% <0.00%> (ø)
protocol_tests/aws-json/runtimeConfig.ts 100.00% <0.00%> (ø)
packages/property-provider/src/memoize.ts 100.00% <0.00%> (ø)
protocol_tests/aws-query/runtimeConfig.ts 100.00% <0.00%> (ø)
protocol_tests/aws-ec2/EC2ProtocolClient.ts 92.30% <0.00%> (ø)
protocol_tests/aws-restxml/runtimeConfig.ts 100.00% <0.00%> (ø)
protocol_tests/aws-restjson/runtimeConfig.ts 100.00% <0.00%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e2614e...e11cef7. Read the comment docs.

@trivikr trivikr requested a review from alexforsyth June 19, 2020 19:50
Copy link
Contributor

@alexforsyth alexforsyth left a comment

Choose a reason for hiding this comment

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

LGTM. Minor comments to be addressed

@trivikr trivikr force-pushed the retry-config-provider-use-middleware-pr branch from d1a896b to e11cef7 Compare June 23, 2020 04:01
@trivikr trivikr merged commit 8f3fdc0 into aws:master Jun 23, 2020
@trivikr trivikr deleted the retry-config-provider-use-middleware-pr branch June 23, 2020 06:09
@AllanZhengYP

This comment has been minimized.

@trivikr

This comment has been minimized.

@trivikr

This comment has been minimized.

@AllanZhengYP

This comment has been minimized.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants