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 Rate Limits in Octocrab #349

Open
programLyrique opened this issue Apr 21, 2023 · 10 comments
Open

Handle Rate Limits in Octocrab #349

programLyrique opened this issue Apr 21, 2023 · 10 comments
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed

Comments

@programLyrique
Copy link

I sometimes get hit by secondary rate limits when searching code and I would like to know how long exactly I should wait before retrying (I am doing exponential backoff currently...).
According to the github documentation, I should try after retry-after seconds if this is the response header, or the x-ratelimit-reset header.

Is there a way to retrieve that in octocrab?
I would have expected to find it in the errors field of GithubError but errors is always None in the case of secondary rate limits for me.

@XAMPPRocky
Copy link
Owner

Thank you for your issue!

Personally I don't think this should be something that users have to worry about for the most part. It's an implementation detail of the client. Our current handling of errors should also do this for you.

@jrozner
Copy link

jrozner commented Apr 24, 2023

I've also been running into this not just with Octocrab but also with the official Ruby library. I think there is something specific with the search API that is different.

@programLyrique
Copy link
Author

There is actually specific rate limits for search: https://docs.github.com/en/rest/search?apiVersion=2022-11-28#about-search

@XAMPPRocky XAMPPRocky added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Apr 24, 2023
@XAMPPRocky
Copy link
Owner

So just to clarify for anyone interested. I would accept a PR that updates the get / post / delete / put methods in the octocrab client with a retry task that on 403 errors, checks the x-ratelimit headers, and sets up a delay to retry based on those headers. https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#rate-limiting

@programLyrique I should have also mentioned there is a ratelimit method on Octocrab to get your rate limits manually. This endpoint does not impose rate limits, so you can always check it.

@XAMPPRocky XAMPPRocky changed the title Secondary rate limits: getting how long to wait Handle Rate Limits in Octocrab Apr 24, 2023
@programLyrique
Copy link
Author

programLyrique commented Apr 24, 2023

It seems that ratelimit endpoint just shows the primary rate limits, not the secondary ones (and I don't think Github provides any more information than the x-ratelimit about the secondary ones).

Here is a request I do immediatly after getting hit by a secondary rate limit error (and just before another one, as my exponential backoff has not reached a long enough duration):

RateLimit { resources: Resources { core: Rate { limit: 5000, used: 0, remaining: 5000, reset: 1682344037 }, 
search: Rate { limit: 30, used: 0, remaining: 30, reset: 1682340497 }, 
graphql: Some(Rate { limit: 5000, used: 0, remaining: 5000, reset: 1682344037 }), integration_manifest: Some(Rate { limit: 5000, used: 0, remaining: 5000, reset: 1682344037 }), 
scim: Some(Rate { limit: 15000, used: 0, remaining: 15000, reset: 1682344037 }), 
source_import: Some(Rate { limit: 100, used: 0, remaining: 100, reset: 1682340497 }), 
code_scanning_upload: Some(Rate { limit: 1000, used: 0, remaining: 1000, reset: 1682344037 }), actions_runner_registration: Some(Rate { limit: 10000, used: 0, remaining: 10000, reset: 1682344037 }) }, 
rate: Rate { limit: 5000, used: 0, remaining: 5000, reset: 1682344037 } }

As you can see, for search, it actually shows 0 request used, and 30 remaining (I would have expected to see at least 1 request used as I had a successful request).

I will have a look at how to add retrying based on x-ratelimit and try to make a PR...

@XAMPPRocky
Copy link
Owner

@programLyrique I actually forgot that someone did implement this already but I haven't accept the PR 😅

#310

@jrozner
Copy link

jrozner commented Apr 24, 2023

I have some code that I wrote that used this API and simply added increasing wait times up until 1 minute in between page requests and was never able to get it to succeed after the first page. It always hit the secondary rate limit. Have you been able to get it to succeed? I can't imagine needing to wait longer than 1 minute.

@programLyrique
Copy link
Author

programLyrique commented Apr 25, 2023

@XAMPPRocky

I actually forgot that someone did implement this already but I haven't accept the PR sweat_smile

#310

Yes, I actually saw it and had found a feature flag now with some retry capability that can be specified with add_retry_config but had not seen the new comments on that PR.

I think the best would be to use the code that extracts the headers of that PR, and add them in the retry of that file.
I will wait to see if @goodspark does it!

@programLyrique
Copy link
Author

programLyrique commented Apr 25, 2023

@jrozner

I have some code that I wrote that used this API and simply added increasing wait times up until 1 minute in between page requests and was never able to get it to succeed after the first page. It always hit the secondary rate limit. Have you been able to get it to succeed? I can't imagine needing to wait longer than 1 minute.

The ratelimit API does not give me usable results as even when the error is that I used all the resources, I always have the maximum number of requests remaining. So I just implemented an exponential backoff with an arbitrary base duration (30s) I think, and usually, after 2 or 3 retries, it worked. I also always waited 1s between every request. The rate limit is over 1h so you might have to wait for 1h if you do too many requests in a short burst.

@goodspark
Copy link
Contributor

Search is a bit different. It resets a lot faster (depending on if your request is authenticated). https://docs.github.com/en/rest/search?apiVersion=2022-11-28#rate-limit So it makes sense it would work after 2-3 retries if you start at 30s (bc generally you'll be over 1min by then).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants