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 informative parameters to 'RateLimited' Exception #211

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlonFasta
Copy link

@AlonFasta AlonFasta commented Nov 3, 2020

@jsocol in order to allow proper differentiation and documentation in case of multiple rate limit decorators on a single Django view, i have added 'key', 'group' and 'rate' details to 'RateLimited' Exception, WDYT?

Thanks :)

…order to allow proper differentiation and documentation in case of multiple rate limit decorators on the same django view.
@AlonFasta
Copy link
Author

@jsocol Any feedback?

@@ -20,7 +20,7 @@ def _wrapped(request, *args, **kw):
increment=True)
request.limited = ratelimited or old_limited
if ratelimited and block:
raise Ratelimited()
raise Ratelimited(group=group, key=key, rate=rate)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why stop there? How about allow users to give a message for Ratelimit (helpful if you've got a middleware that can't differentiate views).

@aambrozkiewicz
Copy link

Hey, I had similar idea but I would say than instead of passing individual parameters, why not pass what get_usage returns? This way people might use the additional information like time_left as part of the response.

Let me know, I can prepare a PR.

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.

3 participants