Skip to content

feat!: introduce rate limiting #42

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

Merged
merged 7 commits into from
May 21, 2020
Merged

Conversation

philnash
Copy link
Contributor

This centralises calls to got in the TwilioServerlessApiClient. Instead of making calls directly on the got client, we now make calls through the TwilioServerlessApiClient's request method. This allows us to add concurrency limiting and retry logic to those calls in a central place.

  • Uses p-limit to limit concurrency with a default level of 50 concurrent calls (Twilio limit is 100)
  • Concurrency is configurable
  • Uses got to retry failed calls with a limit of 10 retries (got only retries specific failures including 429 errors)

Fixes #37.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

philnash added 4 commits May 19, 2020 12:23
This will allow us to control the concurrency of requests made through the client at a later date.
Has a default concurrency of 50, but is configurable in the TwilioServerlessApiClient constructor.
Got already has retry capabilities, with back off, built in. Default number of retries is 2, so this increases to 10. It also only retries on certain HTTP statuses (including 429) and other network errors.
Since we no longer pass the GotClient around, adding the ClientConfig to it doesn't help. Instead, we pass the config to functions that need it.
@philnash philnash requested a review from dkundel May 19, 2020 02:47
@dkundel
Copy link
Contributor

dkundel commented May 20, 2020

Btw it seems also like the docs are failing to generate: https://app.netlify.com/sites/twilio-serverless-api/deploys/5ec348c7d323bd000726519e

@philnash
Copy link
Contributor Author

Don't know what was up with the docs, but we're good now @dkundel.

Also I'll open an issue about splitting the request function into a separate HTTPClient class so we can handle that outside this PR.

@philnash philnash requested a review from dkundel May 21, 2020 02:07
@dkundel dkundel changed the title Client refactor feat!: introduce rate limiting May 21, 2020
@dkundel dkundel merged commit 6dc1bc4 into twilio-labs:master May 21, 2020
@philnash philnash deleted the client-refactor branch May 25, 2020 04:07
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.

hitting '429 Too many requests' when deploying large number of assets
2 participants