Raise a RateLimitExceededException when a secondary rate limit is hit
#2127
Conversation
I have been writing some new code that ended up hitting github's secondary rate limit, and was confused why it was being returned as a generic `GithubException` instead of a `RateLimitExceededException`. I dug into the code, figured out why, and put together this patch. Let me know if there's anything I can do to improve it, especially with the replay data
william-richard
force-pushed
the
will-raise-RateLimitExceededException-when-a-secondary-rate-limit-is-hit
branch
from
ff7b4d3
to
7bcafff
Compare
william-richard
changed the title
RateLimitExceededException when a secondary rate limit is hit
william-richard
changed the title
RateLimitExceededException when a secondary rate limit is hit
william-richard
changed the title
RateLimitExceededException when a secondary rate limit is hit
|
I have not seen your error before: But I have seen these (EnricoMi/publish-unit-test-result-action#170, EnricoMi/publish-unit-test-result-action#150): which are already caught by PyGithub due to the Your message does not match as it misses that string ending. Maybe, besides adding the string starting as you suggest, it is also worth adding the string ending of your message as well to catch other errors that haven't been seen / reported yet and GitHub is likely to come up with: |
| @@ -424,6 +424,9 @@ def __createException(self, status, headers, output): | |||
| or output.get("message") | |||
| .lower() | |||
| .endswith("please wait a few minutes before you try again.") | |||
| or output.get("message") | |||
There was a problem hiding this comment.
Maybe it is more intuitive to move that startswith up to the first startswith to collect startswith conditions and endswith conditions.
| @@ -133,6 +133,17 @@ def exceed(): | |||
|
|
|||
| self.assertRaises(github.RateLimitExceededException, exceed) | |||
|
|
|||
| def testSecondaryRateLimitExceeded(self): | |||
There was a problem hiding this comment.
There are other messages that indicate secondary rate limit, worth testing them as well? #2127 (comment)
|
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 ReportBase: 98.90% // Head: 98.77% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2127 +/- ##
==========================================
- Coverage 98.90% 98.77% -0.13%
==========================================
Files 108 117 +9
Lines 11131 11674 +543
==========================================
+ Hits 11009 11531 +522
- Misses 122 143 +21
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
I have been writing some new code that ended up hitting github's secondary rate limit,
and was confused why it was being returned as a generic
GithubExceptioninstead of aRateLimitExceededException.I dug into the code, figured out why, and put together this patch.
Let me know if there's anything I can do to improve it, especially with the replay data