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 header #522

Closed
mfclarke opened this issue May 17, 2016 · 5 comments
Closed

Rate limit header #522

mfclarke opened this issue May 17, 2016 · 5 comments

Comments

@mfclarke
Copy link

According to the rate limit docs, there's now a header that contains rate limiting information. I'd like to add support for this, but can I request some design suggestion from you @arsduo? Would it make sense to add this information to the GraphCollection and other result objects? It's not really an error per se, but an indicator that an error might occur soon due to rate limiting, so I think it makes sense to pass this onto the result objects as a hash attr.

@batasrki
Copy link

I've already implemented it here, Shopify@266ff05.

I needed it for some of Shopify's FB apps. The tests are failing all over the place, though, so, if someone can help with that, I can put a pull request together.

@danielbonnell
Copy link

Something like this would be fantastic!

@arsduo
Copy link
Owner

arsduo commented Feb 17, 2017

Sorry about the late response. This is a great idea.

There's already support for getting the whole response object, but that's not really useful in this case. Do you think it's better to expose all the headers or just the rate limiting info?

@batasrki, I've fixed the failing tests in master for version 3.0 — if you want to make a PR against that that would be awesome. Thank you!

@batasrki
Copy link

@arsduo as soon as I have some time, I'll make a PR and we can discuss what to return. I think returning the headers in full would be good, as well as adding convenience functions to pull certain useful things out of the headers.

@arsduo
Copy link
Owner

arsduo commented Mar 1, 2017

Good news! #589 was merged, which exposes the headers for all GraphCollection objects. For non-GraphCollection objects, you can use http_component: :response to get the whole Response object.

If you upgrade to 3.0.0.beta3 or above, you can test it out. (There should be few to no breaking changes for most users, even though the internals have changed a bunch.) Let me know how it goes!

@arsduo arsduo closed this as completed Mar 1, 2017
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

4 participants