Switch to retryable http client addresses #546

[tback]

I replaced the default http client with hashicorps retryable httpclient. That client detects and adapts to http status code 429. This is my first PR in this project.

[tback]

Let me know if there is anything I can do to make this any easier on your end.

[scotchneat]

@tback thank you for your contribution. I can see where this will be useful in many cases. I'm working on understanding the impact to the behavior of the provider and need to determine if there are any cases where we'd want to avoid retries before we accept this change.

[tback]

Hi Cesar,

thanks for looking into this. One more thing to add since it might not be obvious from the PR: Hashicorp/go-retryablehttp behaves very well in this situation: it evaluates the retry-after header if available and uses exponential backoff as fallback ( https://pkg.go.dev/github.com/hashicorp/go-retryablehttp?utm_source=godoc#DefaultBackoff ).

On Mon, 9 May 2022, 20:31 Cesar Garza, @.***> wrote:

@tback https://github.com/tback thank you for your contribution. I can see where this will be useful in many cases. I'm working on understanding the impact to the behavior of the provider and need to determine if there are any cases where we'd want to avoid retries before we accept this change.

— Reply to this email directly, view it on GitHub https://github.com/digitalocean/terraform-provider-digitalocean/pull/820#issuecomment-1121438733, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUL2BYW5HPOSIYLQF6MJTVJFKZTANCNFSM5UIXHEXA . You are receiving this because you were mentioned.Message ID: @.*** com>

[scotchneat]

@tback, I was reading into that backoff behavior and think that's one of the great values this change would offer.

I do have some thoughts.

1. It would be practical to allow users to override the retryablehttp client's default values (like the RetryWaitMin, RetryWaitMax, and RetryMax values) to suite their needs. This would require adding some properties to the Config struct, as well as schema values to the provider to load from environment variables.
2. More notable, I'm thinking that a better place to apply this change would be in the digitalocean/godo pacakge as that would enable any go client to leverage the retry capabilities.

[tback]

@scotchneat thanks for your suggestions. They absolutely make sense to me. I won't have time to implement them over the next months though. Anybody can take it from here?

[tback]

Nevermind.