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

Make it easer to detect rate limit errors #152

Closed
imjasonh opened this issue Nov 20, 2014 · 3 comments
Closed

Make it easer to detect rate limit errors #152

imjasonh opened this issue Nov 20, 2014 · 3 comments
Assignees

Comments

@imjasonh
Copy link

The GitHub API returns rate limit information in HTTP response headers for every request.

It might be useful if the GitHub client (optionally) inspected those values and proactively avoided hitting rate limits by blocking a subsequent request until it thinks it will have quota, then checking that it does indeed have quota, before making the request.

This would probably be prohibitively hard to get right 100% of the time, but as long as it left some slush quota in case of race conditions, it could be really useful for clients to automatically obey their rate limit without having to design for it.

@willnorris
Copy link
Collaborator

Yeah, we're already parsing the rate limit info off every response and storing it in client.Rate, so having client.Do() check that before completing shouldn't be hard at all. As you mention, the key is just leaving some wiggle room in case of race conditions. You want to take a stab at this?

@willnorris willnorris reopened this Nov 20, 2014
@willnorris willnorris changed the title Automatically adhere to rate limits Make it easer to detect rate limit errors Jan 21, 2015
@willnorris
Copy link
Collaborator

As discussed in #153, automatically throttling the application is not the right behavior for this library. However, we could make it a little easier for applications to detect rate limit errors (basically a 403 response with zero rate.Remaining) so they can respond appropriately.

@dmitshur
Copy link
Member

As discussed in #153, automatically throttling the application is not the right behavior for this library. However, we could make it a little easier for applications to detect rate limit errors (basically a 403 response with zero rate.Remaining) so they can respond appropriately.

Agreed.

As you mention, the key is just leaving some wiggle room in case of race conditions.

Now that #255 has been done, this should be very doable.

You want to take a stab at this?

I'll take this. PR soon.

It'll be 2 PRs. First one just to add a new error type for rate limit exceeded situations and returning it when appropriate. The second PR will be an optimization (with no change to library behavior) to avoid doing unnecessary networking calls when it's known that the rate limit has been exhausted and not enough time has yet passed for it to be reset.

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

No branches or pull requests

3 participants