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 the API call #29

Closed
rajnmithun opened this issue Aug 29, 2016 · 10 comments
Closed

rate limit the API call #29

rajnmithun opened this issue Aug 29, 2016 · 10 comments
Assignees

Comments

@rajnmithun
Copy link
Contributor

It would be nice to have rate limiter before making a call to API if it is in the local cache.

@dsnet
Copy link
Contributor

dsnet commented Aug 29, 2016

Hi, can you elaborate? Currently, the logic avoids any API calls over the network if the result is in the local cache.

Are you talking about a general rate-limiter for network traffic?

@rajnmithun
Copy link
Contributor Author

I am talking about the network traffic in making the API call.
The scenario I am talking about is, if there are more than 10k unique partial hash hit and these are not in the local cache then we would make API call. At this point if the API key has a rate limit of 10K. It would exceed and get blocked.

@dsnet
Copy link
Contributor

dsnet commented Aug 29, 2016

I see. In your usage of LookupURLs, are you passing in a batch of URLs every time you call it?

Before implementing any rate limiting logic, I would much rather see the TODO in that function be addressed. As it currently is, we make a network call for each URL, rather than batching them together.

On the other hand, if you have many goroutines calling LookupURLs independently with only a small handful of URLs, then some rate limiting mechanism may be needed.

@rajnmithun
Copy link
Contributor Author

I am currently not batching the URLs to LookupURLs(which is on my TODO). Also I have a scenario where I have multiple goroutines/instance of the client running which would still need a mechanism for rate limiting across them.

@rajnmithun
Copy link
Contributor Author

Batching of the URLs is going to be done any time soon or should I submit a PR?

@dsnet
Copy link
Contributor

dsnet commented Aug 30, 2016

You're welcome to submit a PR (assuming you sign the CLA) and I can review it.

@rajnmithun
Copy link
Contributor Author

rajnmithun commented Sep 26, 2016

I have a PR ready for the batching of the partial hashes. Can you please take a look at it @dsnet .

@rajnmithun
Copy link
Contributor Author

@dsnet can you please let me know how to get this reviewed and merged in to master branch.

@alexwoz
Copy link
Collaborator

alexwoz commented Oct 4, 2016

Hi @rajnmithun, I apologize for the delay. I will try to take a look at this as soon as possible!

@rajnmithun
Copy link
Contributor Author

@alexwoz thank you.

@alexwoz alexwoz closed this as completed Oct 17, 2016
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

3 participants