Add retryablehttp Client Option #619
Conversation
| retryableClient.RetryWaitMax = time.Duration(c.RetryConfig.RetryWaitMin * float64(time.Second)) | ||
|
|
||
| // if timeout is set, it is maintained before overwriting client with StandardClient() | ||
| retryableClient.HTTPClient.Timeout = c.client.Timeout |
There was a problem hiding this comment.
I find that some opensource packages will also set a timeout, so decided to maintain it as well.
| // RetryConfig sets the values used for enabling retries and backoffs for | ||
| // requests that fail with 429 or 500-level response codes using the go-retryablehttp client. | ||
| // RetryConfig.RetryMax must be configured to enable this behavior. RetryConfig.RetryWaitMin and | ||
| // RetryConfig.RetryWaitMax are optional, with the default values being 1.0 and 30.0, respectively. |
There was a problem hiding this comment.
RetryWaitMax and RetryWaitMin do have default values: https://github.com/hashicorp/go-retryablehttp/blob/main/client.go#L51
There was a problem hiding this comment.
Those defaults look good based on the testing I did over the course of looking at this branch, so I'm happy with using those 👍
| retryableClient.RetryWaitMin = time.Duration(*c.RetryConfig.RetryWaitMin * float64(time.Second)) | ||
| } | ||
| if c.RetryConfig.RetryWaitMax != nil { | ||
| retryableClient.RetryWaitMax = time.Duration(*c.RetryConfig.RetryWaitMax * float64(time.Second)) |
There was a problem hiding this comment.
An effort to avoid an unset RetryWaitMax and RetryWaitMin with a value of 0.0 overriding the retryablehttp's default value of 30 seconds and 1 second, respectively.
| type RetryConfig struct { | ||
| RetryMax int | ||
| RetryWaitMin *float64 // Minimum time to wait | ||
| RetryWaitMax *float64 // Maximum time to wait |
There was a problem hiding this comment.
Changed RetryWaitMin and RetryWaitMax to pointers to differentiate between an unset variable and a variable set to 0.0.
| // RetryConfig sets the values used for enabling retries and backoffs for | ||
| // requests that fail with 429 or 500-level response codes using the go-retryablehttp client. | ||
| // RetryConfig.RetryMax must be configured to enable this behavior. RetryConfig.RetryWaitMin and | ||
| // RetryConfig.RetryWaitMax are optional, with the default values being 1.0 and 30.0, respectively. |
There was a problem hiding this comment.
Those defaults look good based on the testing I did over the course of looking at this branch, so I'm happy with using those 👍
Users can opt to use the httpclient from the retryablehttp package when creating a godo client to enable automatic retries and exponential backoff.
Users can enable using the retryablehttp httpclient by passing in the
SetRetryMax()option when creating a client usingNew(). Users can then configure the retryWaitMax, and retryWaitMin values.This first iteration uses the retryablehttp's
DefaultRetryPolicyandDefaultBackOff. By not explicitly setting them, they are the defaults as shown here.For the test, I passed in retryMax=2 and retryWaitMax=6.0 seconds and retryWaitMin=6.0s:
As you can see, it retries 2 times and waits 6 seconds inbetween retries. I'm not sure how to explicitly check the test captured the number of retries and time between in this test- so open to suggestions.