test case for dynamic rate limit added #66
Conversation
There was a problem hiding this comment.
Thanks for raising this PR @sandeepkumar-one and apologies for the delay in responding to you, I was afk on holiday 😅
You unearthed a bug in the exempt decorator which I fixed in #68. Thanks for that!
Just one comment on your tests, but otherwise happy to merge this. Nice to see the docs improvements :)
tests/test_fastapi_extension.py
Outdated
|
|
||
| client = TestClient(app) | ||
|
|
||
| for i in range(0, 10): |
There was a problem hiding this comment.
You're not really testing the exemption here, since you run 10 calls on an endpoint that would be limited to 10/minute anyway. I'd suggest lowering the test limit to 2/minute for instance, and run 10 calls to make sure you are indeed exempted.
Also, I fixed the bug you found in exempt in #68. Feel free to test your code with that PR to make sure it works.
There was a problem hiding this comment.
Thanks @laurentS, I have updated the test case.
There was a problem hiding this comment.
Thanks! The CI checks are not passing, it looks like you need to run black on your changes, can you do that?
|
@sandeepkumar-one I've also added you to the repo as per #58 |
Test case for dynamic rate limiting added