-
Notifications
You must be signed in to change notification settings - Fork 37
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
Use sanic-limiter to throttle api requests #235
Conversation
Deploying with Cloudflare Pages
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t know if these rates are permissive enough - the UI polls the API every few seconds
what’s the intent behind this? can we gate just the “submit prompt” endpoint?
yes, we can very well do route based throttling. I thought the UI would not poll more than a certain number of times per minute for the The context for the intent is here though: https://astronomer.slack.com/archives/C05QJA9LTR9/p1703159886303619 (this has been linked in the issue #230 description) |
5096b85
to
4759904
Compare
4759904
to
2a5778b
Compare
LGTM. This will have no impact on the Slack path since it does not make rest API calls, right? |
yes, correct it won't impact Slack calls. However, Slack could impose rate limits by itself based on the limits specified here https://api.slack.com/apis/rate-limits |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Co-authored-by: Ankit Chaurasia <8670962+sunank200@users.noreply.github.com>
This reverts commit 935ee41.
Throttles API requests based on user's IP address.
With this PR, at the moment, we are only throttling
/requests
endpoint.closes: #230