Support for secondary and additional rate limits

[xmo-odoo]

Over the summer, github introduced secondary rate limits.

It'd probably make sense for PGH to have built-in support for those:

• mutation requests can now return a failure (403) with a Retry-After header, indicating how many seconds to wait
• mutation requests should now be interspersed by at least one second
• the more complicated objects (those which trigger notifications e.g. prs, issues, comments, reviews, probably repos) are further restricted but with no indications

Would be nice if PGH could support these at least as an opt in e.g.

• opting into automatically retrying on 403 + Retry-After
• opting into self-rate-limiting on mutation request (could be as simple as a delay_mutation=False which would be used as a number of seconds since the last mutation query, since False == 0 and True == 1 that could directly be interpreted as seconds and would further allow users to tune the rate-limiting more finely)
• opting into a more extensive rate limiting for the more complex class of objects

[karlicoss]

related: #2127

[EnricoMi]

I think PyGithub should throttle all requests so that rate limit errors are avoided in the first place (#1989). If they occur, they should be retried while respecting Retry-After.

And this is where things get complicated:

1. Rate limit errors (403) do not have Retry-After header, at least not in all situations: https://github.community/t/no-retry-after-header/184498/7.
2. Not all 403 errors are retry-able (e.g. access forbidden if not authenticated), so retrying based on status code (403) only would be a bad idea. The urllib3.util.retry.Retry interface does not allow for checking the response message or body content, only response code: Allow Retry class to determine to retry based on response urllib3/urllib3#2500

What I do is create a GitHub instance with a bespoke urllib3.util.retry.Retry class that is able to determine whether to retry based on the response content. With that, 403 responses that refer to RateLimitExceeded are retried, other 403 responses are not retried. The default Retry implementation will then respect Retry-After headers if they exist.

https://github.com/EnricoMi/publish-unit-test-result-action/blob/043296c976c53f4536194ebe3841ed720e04d496/python/publish/retry.py#L17-L76

retry = GitHubRetry(total=retries,
                    backoff_factor=backoff_factor,
                    allowed_methods=Retry.DEFAULT_ALLOWED_METHODS.union({'GET', 'POST'}),
                    status_forcelist=list(range(500, 600)))
github.Github(login_or_token=token, base_url=url, per_page=100, retry=retry)

[xmo-odoo]

I think PyGithub should throttle all requests so that rate limit errors are avoided in the first place (#1989).

"Mitigated" is probably a better qualifier: because github publishes no guidance on notification-triggering operations, can change the rate limiting at any moment, and provides no Retry-After header then, there is no way to ensure the issue is avoided.

[EnricoMi]

"Mitigate" is a better word indeed as we cannot avoid the RateLimitExceededExceptions: #2145

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[cernenwein]

Hitting this error. Is this slated for a fix?