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 Limiting #4

Open
amybarker opened this issue May 23, 2016 · 4 comments
Open

Rate Limiting #4

amybarker opened this issue May 23, 2016 · 4 comments

Comments

@amybarker
Copy link
Contributor

No description provided.

@pwsiegel
Copy link
Contributor

My opinion: the SDK should handle rate limiting simply by raising an exception when the rate limit is exceeded. If we want to handle more complicated logic (e.g. automatically wait for rate limits to reset) then it should be part of a separate "batch operations" package built on top of the SDK, not hard-coded into it. (It's OK if this package ships with the SDK.) My arguments:

  • We want to build the user expectation that the SDK parallels the real API. If the Brandwatch servers throw an exception, so do we.
  • There are numerous issues to be sorted through with batch API operations, and it should be taken seriously as a problem in its own right. For instance, we have a duplication issue in batch get_mentions requests. (We don't have to solve all such problems in one go, but we should have a framework for dealing with them systematically.)
  • The "right" way to handle rate limits depends on context - it's very different in single threaded vs. multithreaded applications, for instance. Throwing an exception provides the best balance of safety and flexibility.

@anthonybu
Copy link
Contributor

I think retrying function should be build in as an option. My thoughts on this come in two folds:

  1. I agree with @pwsiegel that the SDK should be able to reproduce the API behavior.
  2. The fact that this SDK is a helper class for the API, IMHO, should include such functions to reduce development work and provide standard approach for common exceptions like exceeding rate limit. There is no way to stop someone from DoS the API, but at least you can show how the retrying should be done.

@amybarker
Copy link
Contributor Author

@natewalton I know you had been thinking about this problem - looping you in, in case you hadn't seen this issue

@natewalton
Copy link
Contributor

I'm torn, on the one hand I agree with @pwsiegel that the SDK should just do the work of translating the API's response into something Python (e.g. JSON error message --> Exception) and stop there. It most clearly defines what the SDK should, and should not be for.

But I also can't help but think that we'd save a lot of repetitive coding in SDK user's lives by providing options for built-in rate limiting. What I think justifies being in SDK itself is that rate limiting is a standard (and universal) component in API access much like the access token or any other parameter that we would manage on behalf of the user.

It seems like there might be an elegant solution within the SDK, such as a nice decorator? What's the simplest way a user would code against rate limits working on the SDK from the outside? (my brain is tired so maybe this is obvious)

Final thoughts would be that, first, we already have batch operations in the SDK that are combinations of API endpoints, so more complex batching then API rate limiting, and second, I would expect that multi-threaded applications wouldn't be any different a problem for in-SDK rate limiting since the rate limit won't rely on a exogenous counter and rather the JSON error response?

Anyways FWIW!

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

No branches or pull requests

4 participants