-
Notifications
You must be signed in to change notification settings - Fork 449
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
Feature: Setting a rate limit on the endpoints using flask limiter #1006
Comments
Marking this as status on hold because it needs to be approved by @isabelcosta . |
@epicadk this sounds amazing. This is the kind of thing I only learned this year, so I really appreciate you coming with these "out of the box" ideas 🤗 I will approve this. Could you link the library in the description of the issue and add a little more information if you have more. |
@isabelcosta 😅 Thankyou : ) . I have linked the library in the issue however I haven't used it much so I don't really have any more information about it. We also need to discuss the limit or the amount of times that a user should be allowed to query the backend. |
thank you so much @epicadk ! |
I'd like to work on it |
I think we already discussed it. Since Isabel has approved I'll assign you. Happy coding : ) . |
Ohk! Do you mind sharing the meeting docs link? I'd like to go through the discussion. |
I went through the docs and didn't find any thing related to this issue so maybe I was mistaken. But isabel has apporved it so you can work on it. What we need to discuss is the rates for each endpoint. You can still work on it and submit a pr. Although you might have to change only the rates later on so up to you. |
Ok cool I'll start working on it then ;) |
@jalajcodes hey any updates? |
@mtreacy002 I think this issue can cause problems for BIT. we'll have to exclude the BIT server from the rate limiting. |
I'll create PR tomorrow or if time permits, today |
@jalajcodes take your time, just update the progress after every 3 days if it's taking long |
@epicadk , I agree, this could cause issue if the rate limiter sets the amount of requests BIT can do, since in BIT-MS integration the number of requests sent would be extensive 🤣🤣. |
As we haven't decided rates for specific endpoints yet, I am thinking of adding a generous rate limit globally for all endpoints and exempt it for those endpoints which don't need to be rate limited like regarding the BIT-MS integration, I think |
Login is probably the endpoint we want to rate limit first. 😄 . |
@epicadk 😂😅 my bad, just assume any other endpoint that doesn't require limiting |
I would like to take up this issue if it's available. |
I'll create PR today |
Hi @epicadk and @isabelcosta. Please assign this issue to me if it is open :) |
@battuAshita are you still interested in the issue? |
Hi @isabelcosta. I am actually caught up in some work. It is free to be assigned to someone else. Thank you :) |
No worries @battuAshita ! thank you for responding so quickly 🤗 in this way we can assign to someone else who is interested! |
Hi @isabelcosta. Is this open to work on? If yes, can you please assign this to me? Thanks! |
Assigning you @sakshivij |
@vj-codes to decide on rate-limit was there any doc created as mentioned on the closed PR? Or I would need to create one? |
@epicadk can you answer that please? ⬆️ |
That's something we need to discuss in the mentorship session. I think you could probably use a constant right now and then change it later? cc @isabelcosta |
Makes sense. If you mean the sync up session, I'll try to join as well. |
Yes the sync up session, it is conducted on Saturdays biweekly at 6:30 pm IST |
Is your feature request related to a problem? Please describe.
Heroku Does offer DDoS mitigation however, according to this post they strongly recommend using a rate limiter library.
Describe the solution you'd like
Rate limit the endpoints of the api using flask limiter.
Additional context
The time and amount of requests that should be allowed should be discussed, possibly in the mentorship session.
The text was updated successfully, but these errors were encountered: