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

Switch to retryable http client addresses #546 #820

Closed
wants to merge 3 commits into from

Conversation

tback
Copy link

@tback tback commented Apr 25, 2022

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
Copy link
Author

tback commented May 5, 2022

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

@scotchneat
Copy link
Contributor

@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
Copy link
Author

tback commented May 9, 2022 via email

@scotchneat
Copy link
Contributor

@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
Copy link
Author

tback commented May 10, 2022

@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
Copy link
Author

tback commented Nov 18, 2022

Nevermind.

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.

None yet

2 participants