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

Rate limiting mechanism #66

Closed
schelv opened this issue Dec 17, 2023 · 7 comments · Fixed by #163
Closed

Rate limiting mechanism #66

schelv opened this issue Dec 17, 2023 · 7 comments · Fixed by #163
Labels
enhancement New feature or request

Comments

@schelv
Copy link
Contributor

schelv commented Dec 17, 2023

I decided to speed up my algorithm by sending multiple requests at the same time.
It works fine until it doesn't because of the rate limits

It would be nice if rate limits could be taken into account when sending lots of asynchronous requests.
Currently I am encountering secondary rate limits.
The returned response has status 403 and has a 'retry-after' header, maybe it can be used to temporarily stop sending async requests? and continue after the time has passed?

There is also the primary rate limit. Could the headers mentioned be used to coordinate sending the async requests?

@yanyongyu yanyongyu added the enhancement New feature or request label Dec 17, 2023
@yanyongyu
Copy link
Owner

Currently, githubkit is focusing on data validation, auth flows, and maybe api versioning and module lazy loading in the next major version. Rate limit will increase the complexity and may be considered later. At current time, githubkit has implemented the http caching in #61, rate limiting may need to be handled by user.

@schelv
Copy link
Contributor Author

schelv commented Jan 5, 2024

I've implemented rate limiting in my local branch.
However the current implementation is kind of messy, and still requires some improvements.

I can improve the code and make a pull request for this feature.
Is that something you would be interested in?

If so do you have a preference on how the rate limiting should be added?
As a superclass? or as a new configuration option?

@yanyongyu
Copy link
Owner

I think this could be a configuration option and handle retries within the request method

@schelv schelv changed the title Rate limits Rate limiting mechanism Jan 11, 2024
@honzajavorek
Copy link

If #86 is merged, should this still be open? Maybe it's already done?

@yanyongyu
Copy link
Owner

yanyongyu commented Apr 16, 2024

If #86 is merged, should this still be open? Maybe it's already done?

#86 only deal with the rate limit error from the github server. This issue also requests another feature that the github api request concurrency should be controlled. The request concurrency is also mentioned in the github docs and the octokit sdk features.

@malte-behrendt-gr
Copy link

malte-behrendt-gr commented Nov 12, 2024

Thanks for this awesome library!
I'd really like to switch from pygithub.

But I'm seeing errors like this - I assume this is due to rate limits:

  File "XXX/.venv/lib/python3.12/site-packages/githubkit/paginator.py", line 125, in _aget_next_page
    await self.request(
  File "XXX/.venv/lib/python3.12/site-packages/githubkit/versions/v2022_11_28/rest/repos.py", line 327, in async_list_for_org
    return await self._github.arequest(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "XXX/.venv/lib/python3.12/site-packages/githubkit/core.py", line 553, in arequest
    raw_resp = await self._arequest(
               ^^^^^^^^^^^^^^^^^^^^^
  File "XXX/.venv/lib/python3.12/site-packages/githubkit/core.py", line 325, in _arequest
    raise RequestError(e) from e
githubkit.exception.RequestError: Server disconnected without sending a response.

Is there a tweak I can do?
Like adjusting the HTTPX limits or something?

@yanyongyu
Copy link
Owner

@malte-behrendt-gr It seems this exception is not related to the rate limits. Could you please open a new issue with the full error traceback logs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants