-
Notifications
You must be signed in to change notification settings - Fork 371
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
[JENKINS-63078] Allow ignoring rate limits #313
[JENKINS-63078] Allow ignoring rate limits #313
Conversation
src/main/resources/org/jenkinsci/plugins/github_branch_source/Messages.properties
Outdated
Show resolved
Hide resolved
|
||
@Override | ||
public void checkApiRateLimit(@NonNull TaskListener listener, GitHub github) throws IOException, InterruptedException { | ||
// Nothing needed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If pointing at https://github.com
this setting should pass through to ThrottleOnOver
. There is literally never a reason to not check rate limits when interacting with github.com
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we block the selection of NoThrottle
in the config page if the api uri is github.com?
edit: actually scratch that. For some reason I forgot you can add mulitple servers
// there should be no output | ||
assertEquals(0, countOfOutputLines(m -> m.matches("[sS]leeping"))); | ||
// github rate_limit endpoint should not be contacted | ||
githubApi.verify(0, RequestPatternBuilder.newRequestPattern(RequestMethod.GET, urlPathEqualTo("/rate_limit"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks good. One change requested.
2337247
to
3602bf6
Compare
3602bf6
to
7b39521
Compare
I added the requested behaviour. Added a test for it + took the chance to fix a couple wrong regexs from other test assertions. |
Description
This adds ApiRateLimitChecker.NoThrottle, which avoids checking the GitHub rate limits. This is useful for GitHub Enterprise installations. See JENKINS-63078 for further information.
When this change is included, a new strategy will appear on the Github API usage rate limiting strategy selector, called NoThrottle, with the description Ignore rate limit. No requests to the rate_limit endpoint should be performed if it is selected.
Submitter checklist
Reviewer checklist
Documentation changes
Users/aliases to notify