-
Notifications
You must be signed in to change notification settings - Fork 150
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
Handle GitHub Rate Limits During Update Checks #449
Handle GitHub Rate Limits During Update Checks #449
Conversation
Codecov Report
@@ Coverage Diff @@
## main #449 +/- ##
==========================================
- Coverage 93.89% 93.63% -0.27%
==========================================
Files 78 78
Lines 3557 3567 +10
==========================================
Hits 3340 3340
- Misses 217 227 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Two minor remarks, but this looks great already. Thanks a lot for taking care of the bug and for adding the missing headers to the request.
Co-authored-by: Aurelien Gateau <aurelien.gateau@gitguardian.com>
@agateau-gg both changes implemented as requested |
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.
Thanks, merging the changes in.
GGShield checks for updates by connecting to GitHub anonymously. The anonymous API quota is relatively small. For organizations where hundreds of users all connect via a common egress IP, this can result in significant error messages (collectively across users) / confusion during GGShield operations. For example:
This PR address this issue by checking for quota exceeded condition and rescheduling the update check for after the quota has been reset.
Additionally, it adds headers to the update check request that are recommended by GitHub:
User Agent: https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#user-agent-required
GH API Version: https://docs.github.com/en/rest/overview/api-versions?apiVersion=2022-11-28
Media type (accept): https://docs.github.com/en/rest/overview/media-types?apiVersion=2022-11-28