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

Inconsistent rate limit behavior #358

Closed
jeremylow opened this issue Jul 6, 2016 · 2 comments
Closed

Inconsistent rate limit behavior #358

jeremylow opened this issue Jul 6, 2016 · 2 comments

Comments

@jeremylow
Copy link
Collaborator

If the Api isn't instantiated with sleep_on_rate_limit=True, then the Api.rate_limit object doesn't actually decrement the remaining value on any of the end points, even after calling api.InitializeRateLimit(). Since that might be valuable information, it seems like we should retain it. First impression is that the offending code is L4821:

if url and self.sleep_on_rate_limit:
    limit = self.CheckRateLimit(url)

    if limit.remaining == 0:
        try:
            time.sleep(int(limit.reset - time.time()))
        except ValueError:
            pass

Seems to me that the if url and self.sleep_on_rate_limit check should be if url and self.rate_limit and the if limit.remaining should be if limit.remaining and self.sleep_on_rate_limit so that if the user wants to have access to the rate_limit object, they can do so by calling api.InitializeRateLimit() and then they'd have access to the rate_limit object and it would properly keep track of its resource limits.

@jeremylow
Copy link
Collaborator Author

One more place (L4851):

# could just be if url and self.rate_limit:
if url and self.sleep_on_rate_limit and self.rate_limit:
    limit = resp.headers.get('x-rate-limit-limit', 0)
    remaining = resp.headers.get('x-rate-limit-remaining', 0)
    reset = resp.headers.get('x-rate-limit-reset', 0)

    self.rate_limit.set_limit(url, limit, remaining, reset)

@bear
Copy link
Owner

bear commented Jul 6, 2016

Your analysis and solution both make sense - good job!

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

No branches or pull requests

2 participants