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 Rate Limiting Support #100

Closed
thinkingserious opened this issue Nov 8, 2016 · 7 comments
Closed

Add Rate Limiting Support #100

thinkingserious opened this issue Nov 8, 2016 · 7 comments
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap type: question question directed at the library

Comments

@thinkingserious
Copy link
Contributor

We would like to add support for v3 Web API rate limits, as described here.

How would you like this to be implemented? Or please give a thumbs up to express your interest in us implementing to move this task up our queue.

PRs are always welcome as well :)

@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap type: question question directed at the library status: help wanted requesting help from the community labels Nov 8, 2016
@thinkingserious thinkingserious added the difficulty: very hard fix is very hard in difficulty label Oct 1, 2017
@tariq1890
Copy link
Contributor

@thinkingserious . I would like to help with this. But I am not sure I understand this issue completely. Isn't the rate limit already implemented for the SendGrid API? i.e. - https://api.sendgrid.com/v3/resource HTTP/1.1

Or are you looking to implement the rate-limiting logic in the sendgrid-go codebase ?

@thinkingserious
Copy link
Contributor Author

Hi @tariq1890,

We are looking for a solution similar to this one. I hope that helps!

With Best Regards,

Elmer

@leandro-lugaresi
Copy link
Contributor

leandro-lugaresi commented Oct 4, 2017

This isn't a good idea! This is one kind of error that the user must handle.
In some cases, sleep and retry can be a nice way (queues?) but if this is done on a request lifetime the application must handle using another way.

@thinkingserious
Copy link
Contributor Author

Thanks for the feedback @leandro-lugaresi.

Since we have users asking for this, I like the idea of making it optional and off by default.

I also think we should document how this could be handled by the user as you suggest here. Would you be interested in making a PR for documenting best practices? That would at least be a medium task for hacktoberfest.

With Best Regards,

Elmer

@leandro-lugaresi
Copy link
Contributor

leandro-lugaresi commented Oct 4, 2017

I can make this PR but first how we will handle the rate-limit?
If you want the possibility of retry automatically: We can create an option to do this (sleep+retry) and the default would return a specific error type with the information of the rate limit and the user will handle?

@tariq1890
Copy link
Contributor

Yes. If I understand what Elmer said, by default we will just return a 429. The users can a use a flag if they want to sleep and retry on reaching rate limit.

@tariq1890
Copy link
Contributor

@thinkingserious This requires a change in the rest repository rather than sendgrid-go

I also see that there is this issue #94 where you guys are planning on phasing the dependency out. I can continue working on this once issue #94 is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: community enhancement feature request not on Twilio's roadmap type: question question directed at the library
Projects
None yet
Development

No branches or pull requests

4 participants