Retry connection on credential errors #11
Conversation
|
This needs to be polished a bit more and add the capability to set custom |
elmarcoh
force-pushed
the
cred-retry
branch
4 times, most recently
from
92310a7
to
8000e43
Compare
| // `max` attempts with `delay` seconds between requests. | ||
| // This is needed in scenarios where credentials are automatically generated | ||
| // and the program starts before AWS finishes propagating them | ||
| func WithRetries(delay time.Duration, max int) Option { |
There was a problem hiding this comment.
I'm not sure if I'm misunderstanding or not but I think what's happening here is:
- 403s simply apply a delay between retries, though there is no maximum number of retries
- 500s (specific codes are specified by
client.DefaultRetryer) simply specify the maximum number of retries, though the delay is determined by the aws-sdk
Is that right?
There was a problem hiding this comment.
The maximum number of retries is the same for all requests, it's defined by the request.Retryer.MaxRetries() method.
For 403 we simply set a different, longer delay between retries.
Originally 403 statuscodes were not retried, that's the normal DefaultRetrier behavior, thats why in the retryer implementation we have to override the ShouldRetry method.
There was a problem hiding this comment.
Ok I wasn't sure if we should have checked if we exceeded MaxRetries() in the ShouldRetry func or not. I did a bit more digging into the aws-sdk so it looks like that it's not the case. I assume clients of a Retryer will continue calling ShouldRetry until you reach MaxRetries() and then it stops?
There was a problem hiding this comment.
yeah, the MaxRetries() check it's done outside the Retryer implementation/interface
sqs/server.go
Outdated
| } | ||
|
|
||
| // Option is the signature that modifies a `Server` to set some configuration | ||
| type Option func(*Server) (*Server, error) |
There was a problem hiding this comment.
Because this accepts a pointer I don't think we need to return the pointer again, we can omit that and simply return an error.
This is needed in order to compensate for AWS eventually consistent credentials: https://docs.aws.amazon.com/IAM/latest/UserGuide/troubleshoot_general.html Also implements variadic function options for `sqs.NewServer`
This is needed in order to compensate for AWS eventually consistent
credentials:
https://docs.aws.amazon.com/IAM/latest/UserGuide/troubleshoot_general.html