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

Configure Godo Retryable HTTP Client #1016

Merged
merged 20 commits into from
Aug 22, 2023
Merged

Configure Godo Retryable HTTP Client #1016

merged 20 commits into from
Aug 22, 2023

Conversation

danaelhe
Copy link
Member

@danaelhe danaelhe commented Aug 8, 2023

Update terraform-provider to use godo's retryable http client.

When the retryable feature is not set or set to 0 (DIGITALOCEAN_HTTP_RETRY_MAX=0 ) and you reach the 250 req/min threshold- you are met with a 429 error:

│ Error: Error retrieving Database User: GET https://api.digitalocean.com/v2/databases/f7ce-4843-a0d9/users/86-app: 429 (request "adfaa613-e244-4cb1-aa4d-bd82d14bef7b") Too many requests

When the retryable feature is set (DIGITALOCEAN_HTTP_RETRY_MAX=2 ) and you reach the 250 req/min threshold- the client performs automatic retries and exponential backoff and continues:

2023-08-08T15:13:01.396-0400 [DEBUG] provider.terraform-provider-digitalocean: 2023/08/08 15:13:01 [DEBUG] GET https://api.digitalocean.com/v2/databases/7d76-48e3-/users/173-app (status: 429): retrying in 44s (3 left)

The retryable feature to be on by default, with 4 tries.

@danaelhe
Copy link
Member Author

danaelhe commented Aug 8, 2023

I'm curious if we should make this the default behavior (maybe set DIGITALOCEAN_HTTP_RETRY_MAX=2 as default).

@danaelhe danaelhe changed the title Draft: Configure Godo Retryable HTTP Client Configure Godo Retryable HTTP Client Aug 10, 2023
@danaelhe
Copy link
Member Author

danaelhe commented Aug 10, 2023

I'm curious if we should make this the default behavior (maybe set DIGITALOCEAN_HTTP_RETRY_MAX=2 as default).

After team discussion, I've made the retryable feature to be on by default with DIGITALOCEAN_HTTP_RETRY_MAX=4.

@danaelhe danaelhe marked this pull request as ready for review August 10, 2023 16:09
@danaelhe danaelhe requested a review from a team August 10, 2023 16:09
Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Raising this to the top in case you missed my comment in the thread.

With the changes we've made to godo, we should be able to remove this bit:

client.Transport = &oauth2.Transport{
	Base:   client.Transport,
	Source: oauth2.ReuseTokenSource(nil, tokenSrc),
}

@danaelhe
Copy link
Member Author

Raising this to the top in case you missed my comment in the thread.

With the changes we've made to godo, we should be able to remove this bit:

client.Transport = &oauth2.Transport{
	Base:   client.Transport,
	Source: oauth2.ReuseTokenSource(nil, tokenSrc),
}

oh yes thank you! Missed this somehow.

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

🎉 LGTM!

@danaelhe danaelhe merged commit 364fc6c into main Aug 22, 2023
@danaelhe danaelhe deleted the godo_retryable_client branch August 22, 2023 18:51
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