Skip to content
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 limiting #1065

Closed
dbosk opened this issue Aug 29, 2022 · 11 comments · Fixed by #1067
Closed

Handle GitHub rate limiting #1065

dbosk opened this issue Aug 29, 2022 · 11 comments · Fixed by #1067
Labels
bug Something isn't working priority:critical

Comments

@dbosk
Copy link

dbosk commented Aug 29, 2022

I usually hit GitHub's secondary rate limit.

  1. This is not handled well by repobee, seems like it's changed into a 404, or am I missing anything:
[ERROR] NotFoundError: 404 {"message": "Not Found", "documentation_url":
"https://docs.github.com/enterprise/3.4/rest/reference/repos#get-a-repository"
}
[ERROR] Critical exception
Traceback (most recent call last):
  File
"/root/.repobee/env/lib/python3.8/site-packages/_repobee/ext/defaults/github.p
y", line 85, in _try_api_request
    yield
  File
"/root/.repobee/env/lib/python3.8/site-packages/_repobee/ext/defaults/github.p
y", line 246, in create_repo
    repo = self._org.create_repo(name, **kwargs)  # type: ignore
  File
"/root/.repobee/env/lib/python3.8/site-packages/github/Organization.py", line
578, in create_repo
    headers, data = self._requester.requestJsonAndCheck(
  File "/root/.repobee/env/lib/python3.8/site-packages/github/Requester.py",
line 353, in requestJsonAndCheck
    return self.__check(
  File "/root/.repobee/env/lib/python3.8/site-packages/github/Requester.py",
line 378, in __check
    raise self.__createException(status, responseHeaders, output)
github.GithubException.RateLimitExceededException: 403 {"documentation_url":
"https://docs.github.com/en/free-pro-team@latest/rest/overview/resources-in-th
e-rest-api#secondary-rate-limits", "message": "You have exceeded a secondary
rate limit. Please wait a few minutes before you try again."}

During handling of the above exception, another exception occurred:
[...]
  1. According to the documentation on GitHub's secondary rate limit, GitHub returns a respone header Retry-After containing the number of seconds to wait. It would be great if repobee output a warning about the rate limit and then waited and retried.
@slarse
Copy link
Collaborator

slarse commented Aug 30, 2022

This was brought up earlier this year in #991 (when the secondary rate limit was very newly introduced). I tried a simple approach with just respecting the default 1 second rate limit of modify requests (see #996).

This is not handled well by repobee, seems like it's changed into a 404, or am I missing anything:

That 404 is coming from the underlying GitHub API library, it's logged before the 403. The thing that follows "Critical exception" is what caused RepoBee to exit. The chain of events looks a little sketchy (I would expect a 404 to come after the rate limiting which might invalidate the credentials used).

I don't think the Retry-After header was even provided back then, but I might have just glossed over it. Regardless it's not entirely easy to solve as the actual HTTP requests are made by a library (which does not handle rate limiting automatically, unfortunately, see PyGithub/PyGithub#1989).

This is solvable; I just need to wrap every individual call to the PyGithub library in a function that handles rate limits. Solvable but darned inconvenient :)

@slarse slarse added bug Something isn't working priority:critical labels Aug 30, 2022
@slarse
Copy link
Collaborator

slarse commented Aug 31, 2022

Actually, after having slept on it, I can probably handle this on the HTTP layer in just about as shifty a way as I did with the 1-second limit for modify requests.

@dbosk
Copy link
Author

dbosk commented Aug 31, 2022 via email

@dbosk
Copy link
Author

dbosk commented Aug 31, 2022

Do you think that there is a possibility that GitHub rate limiting can cause some students' repos to be empty, that rate limiting kicked in after one API call but before another? (I have found that some students' repos are empty, although they're created with the same repobee command as all the others. Not sure what has gone wrong.)

@slarse
Copy link
Collaborator

slarse commented Aug 31, 2022

I don't really know what effect the rate limiting would have. The repos aren't populated through the API, rather that happens just using Git. While that might also potentially be rate limited, it would take some other form. So maybe? I don't have the exact flow of how repo creation and pushing occurs in my head.

I've a potential fix for the API rate limiting in #1067. It's failing CI due to some outdated pylint rules acting up, but I don't have time to fix that right now. But you could try to check out that branch and see if it works for you. There's really no way for me to try it out in practice, the best I can do is unit test it.

@slarse
Copy link
Collaborator

slarse commented Aug 31, 2022

Giant face palm, I see now that I actually never made a release containing the initial rate limit fix. So I'll get this further rate limit fix merged ASAP and then I'll make a release tomorrow.

@slarse
Copy link
Collaborator

slarse commented Aug 31, 2022

Probably, just by using the latest version of master you shouldn't run into the secondary rate limit to begin with. RepoBee will be a lot more gentle on the modify requests. You can of course also get the latest version from docker.

@slarse
Copy link
Collaborator

slarse commented Aug 31, 2022

@dbosk Patched and deployed to dockerhub. Try the latest version and see if it works for you.

@slarse
Copy link
Collaborator

slarse commented Sep 1, 2022

Aaand of course, after implementing it myself from scratch, I realize that urllib supports Retry-After headers. 🤦‍♂️

I'll incorporate that when I have some time left over, but in the meantime I think my hacked up solution should do the trick.

@slarse
Copy link
Collaborator

slarse commented Sep 2, 2022

@dbosk As soon as you confirm that the latest version of RepoBee solves your problem, I'll make a release of it :)

@dbosk
Copy link
Author

dbosk commented Sep 5, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:critical
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants