feat: Add support to enable backoff when rate limited by the Github API#7192
feat: Add support to enable backoff when rate limited by the Github API#7192andrewhibbert wants to merge 28 commits into
Conversation
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
|
Thank you for your contribution! 🙏 Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected. While you are waiting, make sure to:
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient. Learn more about our contribution guide. |
![semgrep-app[bot]](https://avatars.githubusercontent.com/in/60555?s=60&v=4){kind=small}
| if retryAfterStr := header.Get("Retry-After"); retryAfterStr != "" { | ||
| retrySeconds, err := strconv.Atoi(retryAfterStr) | ||
| if err != nil { | ||
| return RateLimit{}, fmt.Errorf("failed to parse Retry-After header. Returned error: %s", err.Error()) |
There was a problem hiding this comment.
Semgrep found a possible error wrong wrapped.
🥳 Fixed in commit 9e21288 🥳
| resetStr := header.Get("X-RateLimit-Reset") | ||
| reset, err := strconv.ParseInt(resetStr, 10, 64) | ||
| if err != nil { | ||
| return RateLimit{}, fmt.Errorf("failed to parse X-RateLimit-Reset header. Returned error: %s", err.Error()) |
There was a problem hiding this comment.
Semgrep found a possible error wrong wrapped.
🥳 Fixed in commit 9e21288 🥳
| remainingStr := header.Get("X-RateLimit-Remaining") | ||
| remaining, err := strconv.Atoi(remainingStr) | ||
| if err != nil { | ||
| return RateLimit{}, fmt.Errorf("failed to parse X-RateLimit-Remaining header. Returned error: %s", err.Error()) |
There was a problem hiding this comment.
Semgrep found a possible error wrong wrapped.
🍰 Fixed in commit 9e21288 🍰
Signed-off-by: andrewhibbert <[email protected]>
Signed-off-by: andrewhibbert <[email protected]>
zroubalik
left a comment
zroubalik
left a comment
There was a problem hiding this comment.
Could you please open a docs PR, kedacore/keda-docs#1557 seems to be closed
| - **Elasticsearch Scaler**: Support IgnoreNullValues at Elasticsearch scaler ([#6599](https://github.com/kedacore/keda/pull/6599)) | ||
| - **GitHub Scaler**: Add support to use ETag for conditional requests against the Github API ([#6503](https://github.com/kedacore/keda/issues/6503)) | ||
| - **GitHub Scaler**: Filter workflows via query parameter for improved queue count accuracy ([#6519](https://github.com/kedacore/keda/pull/6519)) | ||
| - **Github Scaler**: Implement backoff when receive rate limit errors ([#6643](https://github.com/kedacore/keda/issues/6643)) |
There was a problem hiding this comment.
This is not the right location, please add this entry to unreleased section.
There was a problem hiding this comment.
I have moved this to unreleased section
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Signed-off-by: andrewhibbert <[email protected]>
Raised kedacore/keda-docs#1691 |
There was a problem hiding this comment.
Looking really nice,m great job! My only question at this point is if calling this backoff still makes sense as it's not a backoff but an intelligent 429 management. And the other question that comes to my mind if this should be optional or built-in directly
Signed-off-by: Rick Brouwer <[email protected]>
zroubalik
added
the
waiting-author-response
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
github-actions
Bot
added
the
stale
|
This pull request has been automatically closed due to inactivity. |
4 participants
Adds support to backoff when a rate limit error occurs as per handle rate limit errors appropriately:
Fixes #6643
Relates to kedacore/keda-docs#1691