Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-71849

Request to not make an API limit decision for the user

    • 1740.v51d5810e9e8c

      Specifically https://github.com/jenkinsci/github-branch-source-plugin/blob/3f1889ac475b706b31c186e31ca8266a0a847504/src/main/java/org/jenkinsci/plugins/github_branch_source/ApiRateLimitChecker.java#L100-L120

      This impacts our ability to get proper GitHub support. Our company has enterprise support and one of the conditions for us to get higher GitHub App limits is that we actually use up to the limit and show errors of us hitting the limit. GitHub enterprise support worked with us to raise our app limits once but will not raise them again due to our usage profile.

      Because this setting is ignored for github.com (of which we're using hosted GitHub Enterprise). Jenkins throttles API limits despite us configuring limits to be ignored.

      • This produces a terrible developer experience because our users see delays.
      • It has a bad admin experience because we're challenged by GitHub support to hit our limits when this simply isn't possible with the current implementation of the GitHub branch source plugin.

      We tried disabling limit monitoring but the logs state:

      2023-08-17 17:20:00.949+0000 [id=3390317]	INFO	o.j.p.g.ApiRateLimitChecker$RateLimitCheckerAdapter#checkRateLimit: GitHub throttling is disabled, which is not allowed for public GitHub usage, so ThrottleOnOver will be used instead. To configure a different rate limiting strategy, go to "GitHub API usage" under "Configure System" in the Jenkins settings.
      

          [JENKINS-71849] Request to not make an API limit decision for the user

          Sam Gleske added a comment - - edited

          For context, our rate limit on public github.com for our GitHub App is currently 25k req/hr.

          Sam Gleske added a comment - - edited For context, our rate limit on public github.com for our GitHub App is currently 25k req/hr.

          Sam Gleske added a comment -

          From JENKINS-68321, jglick has a solution which would resolve this ticket https://github.com/jenkinsci/github-branch-source-plugin/pull/653

          Sam Gleske added a comment - From JENKINS-68321 , jglick has a solution which would resolve this ticket https://github.com/jenkinsci/github-branch-source-plugin/pull/653

          Sam Gleske added a comment -

          Thanks jglick looks like the fix was released in 1740.v51d5810e9e8c or higher.

          Sam Gleske added a comment - Thanks jglick looks like the fix was released in 1740.v51d5810e9e8c or higher.

          Liam Newman added a comment -

          sag47 jglick 

          I'm concerned this could have consequences beyond the current user.

          As I remember hearing, rate limit checking was originally added to this plugin because github.com decided that Jenkins clients (globally, beyond just the current user or Jenkins instance) were "abusive" and would be heavily penalized by GitHub.com.

          The GitHub REST API best practices make that story plausible:

          If you hit a rate limit, you should stop making requests until after the time specified by the x-ratelimit-reset header. Failure to do so may result in the banning of your integration.

          The way GitHub identifies "an integration" is via user agent which is the same for all Jenkins instances using this plugin.

          Strongly suggest you revert thud change as the potential consequences are catastrophic.

           

           

           

          Liam Newman added a comment - sag47 jglick   I'm concerned this could have consequences beyond the current user. As I remember hearing, rate limit checking was originally added to this plugin because github.com decided that Jenkins clients (globally, beyond just the current user or Jenkins instance) were "abusive" and would be heavily penalized by GitHub.com. The  GitHub REST API best practices  make that story plausible: If you hit a rate limit, you should stop making requests until after the time specified by the  x-ratelimit-reset  header. Failure to do so may result in the banning of your integration. The way GitHub identifies "an integration" is via  user agent  which is the same for all Jenkins instances using this plugin. Strongly suggest you revert thud change as the potential consequences are catastrophic.      

          Jesse Glick added a comment -

          According to the issue description, GH support specifically requested this change, so it sounds like clarification is needed from GH. I suspect the current rate limiting system in this plugin and the client library needs to be rewritten especially in light of the much higher rate limit available for Apps: rather than checking the remaining rate limit and attempting to self-throttle, just make requests unless and until a request is received to back off, and in that case do so. (In the longer term, switching to GraphQL could dramatically decrease the number of requests needed by Jenkins, as could dropping some nonessential functionality that accreted over the years without much thought.)

          Jesse Glick added a comment - According to the issue description, GH support specifically requested this change, so it sounds like clarification is needed from GH. I suspect the current rate limiting system in this plugin and the client library needs to be rewritten especially in light of the much higher rate limit available for Apps: rather than checking the remaining rate limit and attempting to self-throttle, just make requests unless and until a request is received to back off, and in that case do so. (In the longer term, switching to GraphQL could dramatically decrease the number of requests needed by Jenkins, as could dropping some nonessential functionality that accreted over the years without much thought.)

          Jesse Glick added a comment -

          Currently reverted pending discussion.

          Jesse Glick added a comment - Currently reverted pending discussion.

          Sam Gleske added a comment -

          Is the best place to continue discussion in this Jira ticket or in the formerly opened PR?

          Sam Gleske added a comment - Is the best place to continue discussion in this Jira ticket or in the formerly opened PR?

            Unassigned Unassigned
            sag47 Sam Gleske
            Votes:
            2 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated: