Add retry logic when rate limit #1896
Conversation
change message
…DataDog/datadog-api-client-go into add-retry-logic-when-rate-limit
| log.Printf("\n%s\n", string(dump)) | ||
| fmt.Println("Max retries:", maxRetries, " Current retry:", retryCount) | ||
| if retryCount == maxRetries { | ||
| fmt.Println("Max retries reached") |
There was a problem hiding this comment.
I think you should remove this, and log in !shouldRetry under.
There was a problem hiding this comment.
By turning debug on we should dump out the response no matter retry or not
There was a problem hiding this comment.
Yeah I'm just talking about the last log line,
| retryCount := 0 | ||
| for { | ||
| if retryCount == maxRetries { | ||
| ccancel() |
There was a problem hiding this comment.
Is that the right behavior? I would expect the same as the !shouldRetry below.
There was a problem hiding this comment.
If max retry is reached then next request is cancelled and we can exit without returning a response. Which is not a condition covered by shouldRetryRequest.
There was a problem hiding this comment.
AFAIU this will impact the next select, not the request itself? I think it would make sense to handle this in shouldRetryRequest.
There was a problem hiding this comment.
Yep I moved the logic to shouldRetryRequest
Adding enable retry in the API client and tests --------- Co-authored-by: ci.datadog-api-spec <packages@datadoghq.com> Co-authored-by: Thomas Hervé <thomas.herve@datadoghq.com> da0646c
What does this PR do?
This PR implements retrying in the api client as well as a new test function and test cassette
Review checklist
Please check relevant items below:
This PR includes all newly recorded cassettes for any modified tests.
This PR does not rely on API client schema changes.
Or, this PR relies on API schema changes and this is a Draft PR to include tests for that new functionality.