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

Added code for handling rate limits #186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

n0rtr0n
Copy link

@n0rtr0n n0rtr0n commented Oct 27, 2016

I'm checking for status code 429 in the handleReponse() method, which indicates that the rate limit has been reached, and simply delaying and then repeating the request after the allotted time from the 'X-Retry-After' header returned in the response, up to a maximum of 3 times for now. This has been tested locally using a script that generates over 9000 getTime() requests using OAuth authentication on a development store.

@coveralls
Copy link

coveralls commented Oct 27, 2016

Coverage Status

Coverage decreased (-1.4%) to 67.764% when pulling 4e2d57a on nortronthered:feature/handle-api-rate-limits into 71c6d49 on bigcommerce:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 67.764% when pulling 4e2d57a on nortronthered:feature/handle-api-rate-limits into 71c6d49 on bigcommerce:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 67.764% when pulling 4e2d57a on nortronthered:feature/handle-api-rate-limits into 71c6d49 on bigcommerce:master.

@@ -3,6 +3,9 @@ composer.lock
*.sh
script/*

# IDE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not required, you should be able to add IDE specific ignores to your global .gitignore and not the projects gitignore.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bookernath
Copy link
Contributor

@nortronthered please see our adjustments to rate limiting headers here:

https://developer.bigcommerce.com/api/#rate-limits-oauth

This may change your approach, hopefully for the better!

@lord2800
Copy link
Contributor

lord2800 commented May 4, 2017

I'd feel more comfortable with this if there was a setting to turn this on or off.

@mrself
Copy link

mrself commented Oct 3, 2017

Why have this not merged yet?

@lord2800
Copy link
Contributor

lord2800 commented Oct 3, 2017

A couple of reasons:

  1. There's an outstanding comment about removing the IDE-specific ignores.
  2. There needs to be a toggle to turn this auto-retry behavior off.
  3. We've made changes to how the rate limiting headers behave, which should help improve this code.

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

Successfully merging this pull request may close these issues.

8 participants