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

Allow configuration of dynamodb storage to specify the max retries of aws sdk #4115

Merged
merged 5 commits into from
Mar 19, 2018
Merged

Conversation

rin1221
Copy link
Contributor

@rin1221 rin1221 commented Mar 9, 2018

This was an issue if you are wanting errors to manifest themselves in vault as the errors weren't being shown. By setting the variable to 0 you are able to get error messages from the sdk calls if it fails once and then the clients can handle the retry logic.

@jefferai
Copy link
Member

AWS has a default retry value; if the config value isn't set it should use the AWS default, not zero.

Set the value of the Dynamodb MaxRetries to -1 as the aws sdk uses the default value when that is given as stated in the MaxRetries comments in the following link. https://docs.aws.amazon.com/sdk-for-go/api/aws/#Config
@rin1221
Copy link
Contributor Author

rin1221 commented Mar 14, 2018

I agree. I will update the branch to reflect that.

if dynamodbMaxRetryString == "" {
dynamodbMaxRetryString = conf["dynamodb_max_retries"]
if dynamodbMaxRetryString == "" {
dynamodbMaxRetryString = "-1"
Copy link
Member

Choose a reason for hiding this comment

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

Please use the aws sdk const name, not the value.

@jefferai jefferai added this to the 0.9.6 milestone Mar 19, 2018
@jefferai
Copy link
Member

Thanks!

@jefferai jefferai merged commit af974c2 into hashicorp:master Mar 19, 2018
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.

2 participants