Use RATELIMIT_EXCEPTION_CLASS for configuration #247
Conversation
| cls = getattr( | ||
| settings, 'RATELIMIT_EXCEPTION_CLASS', Ratelimited) | ||
| raise (import_string(cls) if isinstance(cls, str) else cls)() |
There was a problem hiding this comment.
Agree with doing this in general, as it makes testing far easier, but I also hear you on the performance concern. One option would be to move this into the top-level of the decorator function, which gets called far less often—though I admit I'm not positive that settings would always be initialized already, depending when it's executed.
There was a problem hiding this comment.
I think making a global variable makes the most sense where the initial variable is set to None. On dev reload, a new thread is created by the web server, so a django setting "reload" would essentially be a new web server and thus the global variable will be reset to None.
The only concern would be when this package needs to support async caching and ORM. But that's a problem for another time.
I've also got classes tomorrow, so probably won't be able to get to this until next week.
jsocol
force-pushed
the
patch-2
branch
2 times, most recently
from
f0a4a78
to
bf69fef
Compare
|
Thanks for this @Andrew-Chen-Wang! |
Fixes #242
A concern I have is performance. This seems like it is importing every time the view is called. I've implemented this like the rest of the package. However, I feel like this would be better if we instead made a "proxy" or global variable that we saved on first load of the class. The only problem I suppose would be dev reload.