Skip to content

Throttle requests to mitigate RateLimitExceededExceptions#2145

Merged
EnricoMi merged 16 commits intoPyGithub:mainfrom
EnricoMi:branch-throttled-requester
Jun 28, 2023
Merged

Throttle requests to mitigate RateLimitExceededExceptions#2145
EnricoMi merged 16 commits intoPyGithub:mainfrom
EnricoMi:branch-throttled-requester

Conversation

@EnricoMi
Copy link
Collaborator

@EnricoMi EnricoMi commented Jan 10, 2022

This introduces a throttling of requests to the Github REST API (1s for writes, 0.25s for reads, configurable) to mitigate secondary rate limit errors and comply with Github's best practices: https://docs.github.com/en/rest/guides/best-practices-for-integrators?apiVersion=2022-11-28#dealing-with-secondary-rate-limits

It defines a minimum time span between consecutive requests to the GitHub API, both caused by user code calling into the PyGithub API and called by PyGithub internally (e.g. user code iterating over a paginated list). This is achieved by hooking into the lowest possible method that is used to send all requests to GitHub API: Requester.__requestRaw.

For each verb the timestamp of the last request (when __requestRaw returned) is recorded and used before the next request to compute the time that has to be waited before the request is issued. It then calls into time.sleep() to respect the minimum time span.

It distinguishes between the time between any request and the time between writes. GitHub expects at least one second between writing requests (e.g. POST): https://docs.github.com/en/rest/guides/best-practices-for-integrators#dealing-with-secondary-rate-limits

This code is being used in EnricoMi/publish-unit-test-result-action and runs thousands of times each day.

Here is an example log:

2022-10-25 07:49:05 - sleeping 0.1404111385345459s before next GitHub request
2022-10-25 07:49:05 - GitHub request: POST /repos/EnricoMi/publish-unit-test-result-action/check-runs
2022-10-25 07:49:06 - sleeping 1.9985311031341553s before next GitHub request
2022-10-25 07:49:08 - GitHub request: PATCH /repos/EnricoMi/publish-unit-test-result-action/check-runs/9087539036

This fixes #1989.

@stale
Copy link

stale bot commented Apr 16, 2022

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.

@codecov-commenter
Copy link

codecov-commenter commented Oct 13, 2022

Codecov Report

Patch coverage: 98.43% and project coverage change: -0.01 ⚠️

Comparison is base (0bb72ca) 98.36% compared to head (ce52876) 98.36%.

❗ Current head ce52876 differs from pull request most recent head 410f27f. Consider uploading reports for the commit 410f27f to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2145      +/-   ##
==========================================
- Coverage   98.36%   98.36%   -0.01%     
==========================================
  Files         132      132              
  Lines       13280    13313      +33     
==========================================
+ Hits        13063    13095      +32     
- Misses        217      218       +1     
Impacted Files Coverage Δ
github/Requester.py 97.36% <98.27%> (-0.08%) ⬇️
github/Consts.py 100.00% <100.00%> (ø)
github/GithubIntegration.py 97.22% <100.00%> (+0.07%) ⬆️
github/MainClass.py 99.31% <100.00%> (+<0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from 20eb0e8 to aaad2dc Compare October 26, 2022 16:23
@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from ce87a2c to 1868f80 Compare December 22, 2022 07:05
@EnricoMi
Copy link
Collaborator Author

@sfdye this fixes #1989, one of the most favoured PyGithub issues, please consider this with priority during your next stint. Thanks!

@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch 3 times, most recently from abe0438 to edf9ef9 Compare February 17, 2023 10:13
@EnricoMi
Copy link
Collaborator Author

@sfdye @s-t-e-v-e-n-k rebased and conflicts resolved, check it while it's hot!

@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from edf9ef9 to 35274c8 Compare February 17, 2023 10:18
@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch 2 times, most recently from 51cfbda to d5c8fac Compare March 21, 2023 07:45
@EnricoMi EnricoMi added this to the Version 1.59.0 milestone Jun 13, 2023
@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from d5c8fac to a03bc58 Compare June 13, 2023 20:32
Comment on lines +1 to +3
# this file is needed so that tox -e lint does not complain about:
# tests/Requester.py:63: error: Incompatible types in assignment (expression has type "float", base class "BasicTestCase" defined the type as "None")
# tests/Requester.py:64: error: Incompatible types in assignment (expression has type "float", base class "BasicTestCase" defined the type as "None")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't fix this in BasicTestCase?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try to rework this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved into the .py file.

@EnricoMi
Copy link
Collaborator Author

Tested this branch in https://github.com/EnricoMi/publish-unit-test-result-action/actions/runs/5271056850/jobs/9531604524#step:5:586

2023-06-14 19:08:22 +0000 - github.Requester - DEBUG - GET https://api.github.com/repos/EnricoMi/publish-unit-test-result-action …
2023-06-14 19:08:22 +0000 - github.Requester - DEBUG - sleeping 0.15410900115966797s before next GitHub request
2023-06-14 19:08:23 +0000 - github.Requester - DEBUG - POST https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/check-runs …
2023-06-14 19:08:23 +0000 - github.Requester - DEBUG - sleeping 1.9984159469604492s before next GitHub request
2023-06-14 19:08:26 +0000 - github.Requester - DEBUG - PATCH https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/check-runs/14267722611 …
2023-06-14 19:08:26 +0000 - github.Requester - DEBUG - sleeping 1.9983429908752441s before next GitHub request
2023-06-14 19:08:30 +0000 - github.Requester - DEBUG - PATCH https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/check-runs/14267722611 …
2023-06-14 19:08:30 +0000 - github.Requester - DEBUG - sleeping 1.997204065322876s before next GitHub request
2023-06-14 19:08:33 +0000 - github.Requester - DEBUG - PATCH https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/check-runs/14267722611 …
2023-06-14 19:08:33 +0000 - github.Requester - DEBUG - sleeping 1.9984889030456543s before next GitHub request
2023-06-14 19:08:36 +0000 - github.Requester - DEBUG - PATCH https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/check-runs/14267722611 …
2023-06-14 19:08:36 +0000 - github.Requester - DEBUG - GET https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/pulls/469 …
2023-06-14 19:08:36 +0000 - github.Requester - DEBUG - sleeping 0.24811387062072754s before next GitHub request
2023-06-14 19:08:37 +0000 - github.Requester - DEBUG - GET https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/commits/b6956ada953bf773226178eaa4bd63c7c71c3a38 …
2023-06-14 19:08:37 +0000 - github.Requester - DEBUG - sleeping 0.24886202812194824s before next GitHub request
2023-06-14 19:08:37 +0000 - github.Requester - DEBUG - GET https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/commits/b6956ada953bf773226178eaa4bd63c7c71c3a38/check-runs?per_page=1 …
2023-06-14 19:08:37 +0000 - github.Requester - DEBUG - sleeping 0.24963998794555664s before next GitHub request
2023-06-14 19:08:39 +0000 - github.Requester - DEBUG - GET https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/commits/b6956ada953bf773226178eaa4bd63c7c71c3a38/check-runs?per_page=100 …
2023-06-14 19:08:39 +0000 - github.Requester - DEBUG - sleeping 0.21327781677246094s before next GitHub request
2023-06-14 19:08:41 +0000 - github.Requester - DEBUG - GET https://api.github.com/repositories/290227969/commits/b6956ada953bf773226178eaa4bd63c7c71c3a38/check-runs?per_page=100&page=2 …
2023-06-14 19:08:41 +0000 - github.Requester - DEBUG - sleeping 0.22020792961120605s before next GitHub request
2023-06-14 19:08:41 +0000 - github.Requester - DEBUG - GET https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/check-runs/14263319733/annotations?per_page=100 …
2023-06-14 19:08:41 +0000 - github.Requester - DEBUG - sleeping 0.24761605262756348s before next GitHub request
2023-06-14 19:08:42 +0000 - github.Requester - DEBUG - GET https://api.github.com/repositories/290227969/check-runs/14263319733/annotations?per_page=100&page=2 …
2023-06-14 19:08:42 +0000 - github.Requester - DEBUG - sleeping 0.24695897102355957s before next GitHub request
2023-06-14 19:08:42 +0000 - github.Requester - DEBUG - GET https://api.github.com/repositories/290227969/check-runs/14263319733/annotations?per_page=100&page=3 …
2023-06-14 19:08:42 +0000 - github.Requester - DEBUG - sleeping 0.21640610694885254s before next GitHub request
2023-06-14 19:08:43 +0000 - github.Requester - DEBUG - POST https://api.github.com/graphql …
2023-06-14 19:08:43 +0000 - github.Requester - DEBUG - sleeping 1.9967451095581055s before next GitHub request
2023-06-14 19:08:46 +0000 - github.Requester - DEBUG - POST https://api.github.com/repos/EnricoMi/publish-unit-test-result-action/issues/469/comments …

@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from 7a6997a to cd3d699 Compare June 14, 2023 19:28
Copy link
Collaborator

@JLLeitschuh JLLeitschuh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from cd3d699 to ce52876 Compare June 28, 2023 18:48
@EnricoMi EnricoMi force-pushed the branch-throttled-requester branch from 410f27f to caff057 Compare June 28, 2023 19:13
@EnricoMi EnricoMi merged commit 9915580 into PyGithub:main Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PyGithub should provide means to throttle API requests

4 participants