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

Add x-app-usage response headers to result #427

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lendrom
Copy link

@lendrom lendrom commented Jul 25, 2018

Add x-app-usage response headers to GraphAPI request result
Facebook API returns x-app-usage header which contains json with percentage usage of the api for various metrics. It would be handy to have access to that data when using this facebook-sdk.
My simple pr just checks for the header in fb api response and adds it to the result returned by GraphAPI instance if available.

FB docs:
https://developers.facebook.com/docs/graph-api/advanced/rate-limiting
It was also reported here:
#314

Add x-app-usage response headers to GraphAPI request result
Copy link
Member

@martey martey left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. Would you be able to update the changelog and add a simple automated test?

@martey
Copy link
Member

martey commented Jul 25, 2018

I've also made changes to the CircleCI configuration so that the automated tests won't automatically fail.

@lendrom
Copy link
Author

lendrom commented Aug 2, 2018

I can add some tests, but I can see the tests are running on the actual FB API. Wouldn't it be better to run them on a mocked version of it or use something like pyvcr? Now I can reach my app's limits just by running the tests.

@martey
Copy link
Member

martey commented Jan 13, 2019

Would it make more sense to just return all headers? This would negate the need to add more code if Facebook adds more useful headers (e.g. X-Page-Usage, etc.).

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 this pull request may close these issues.

2 participants