Skip to content
This repository was archived by the owner on Feb 1, 2021. It is now read-only.

Conversation

@PetrF0X
Copy link
Contributor

@PetrF0X PetrF0X commented Aug 17, 2015

It was a problem for me to watch the api usage (get the data from http headers). Finally I figured it out and here it is. I couldn't think of any concept how to save the data and I didn't need to store them. So I just look at the integers and print them out after every api call. If you have any idea how to make it better and more useful, please tell me.

@PetrF0X
Copy link
Contributor Author

PetrF0X commented Sep 8, 2015

Hi @holm, can you please have a look at this and maybe provide me with an idea how to make it designed properly?

@albertfdp albertfdp self-assigned this Sep 9, 2015
@PetrF0X
Copy link
Contributor Author

PetrF0X commented Oct 3, 2015

Hello @albertfdp, I was thinking about it and finally I came to a decision that it would be the best if there was a base object called e. g. RateLimits which would all the classes that can be returned extend.
What do you think about this approach?

@albertfdp
Copy link
Contributor

The current approach seems fine for me: I am happy to merge it to master. What are you looking to achieve with what you are proposing now?

@PetrF0X
Copy link
Contributor Author

PetrF0X commented Oct 5, 2015

I'm glad that you accept the current approach.
With this approach the information about the rate limits is decoupled from the returned data. I was thinking if it could be a problem if there was e.g. tens or hundreds of requests per second. The information about the current usage would not be connected to the data so I could maybe warn wrong user about his limit.

I'd be glad if you merged this pull request. We can talk about the other approach later on.

@daniel-sc
Copy link
Contributor

Does this account for response where the headers are not set?
If I remember correctly for PodioFile::get_raw the headers are not set ..

@PetrF0X
Copy link
Contributor Author

PetrF0X commented Oct 5, 2015

@daniel-sc
I'm not familiar with any API call where these headers are not set. But your point is great, if there was such a call, current code would throw a NullPointerException.
I will make another commit which will fix this, soon. Thank you!

@albertfdp
Copy link
Contributor

@daniel-sc 👍 Yes, and some clients do not have rate limit, so the absence of such might be in all requests. Thanks @PetrF0X !

@PetrF0X
Copy link
Contributor Author

PetrF0X commented Oct 5, 2015

@albertfdp @daniel-sc Please have a look at the added code
Thanks

@daniel-sc
Copy link
Contributor

looks fine!

@PetrF0X
Copy link
Contributor Author

PetrF0X commented Oct 11, 2015

@albertfdp Hello, can you please merge this?

albertfdp added a commit that referenced this pull request Oct 12, 2015
Implemented a way how to watch the current api usage (rate limits)
@albertfdp albertfdp merged commit 7e0292b into podio:master Oct 12, 2015
@albertfdp
Copy link
Contributor

Done!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants