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_limit doesn't always return valid information #504

Closed
theckman opened this issue Aug 30, 2014 · 3 comments · Fixed by #536
Closed

rate_limit doesn't always return valid information #504

theckman opened this issue Aug 30, 2014 · 3 comments · Fixed by #536

Comments

@theckman
Copy link
Contributor

If you use Octokit in a long running piece of software with infrequent API calls, the rate_limit Client method can return stale/invalid date.

I've taken a quick glance at the code and it looks like the rate limit information is pulled from the headers of the last request. After some time the information itself becomes stale, and eventually the resets_in field in the Struct becomes a negative number. This happens because time has progressed past the resets_at time, yet the information hasn't been refreshed.

Is there a reason why the /rate_limit API endpoint isn't used?

@pengwynn
Copy link
Collaborator

After some time the information itself becomes stale, and eventually the resets_in field in the Struct becomes a negative number.

Yeah that logic could be smarter.

Is there a reason why the /rate_limit API endpoint isn't used?

For high volume clients, this can save a number of server round trips. A patch to check resets_in vs the current time would be welcome.

@theckman
Copy link
Contributor Author

theckman commented Sep 1, 2014

Thanks for getting back to me.

I'll try to allocate some time to take a look improving the intelligence. Is it OK if I keep this issue open until I, or someone else, provides a PR?

@pengwynn
Copy link
Collaborator

pengwynn commented Sep 2, 2014

@theckman Sure thing.

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

Successfully merging a pull request may close this issue.

2 participants