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

group name isn't correct for CBVs #73

Closed
bitcity opened this issue Aug 3, 2015 · 5 comments
Closed

group name isn't correct for CBVs #73

bitcity opened this issue Aug 3, 2015 · 5 comments

Comments

@bitcity
Copy link
Contributor

bitcity commented Aug 3, 2015

Inside is_ratelimited function, the value of group is always django.utils.decorators.bound_func instead of the actual function name.

This is how I'm using the ratelimit decorator:

rl = method_decorator(ratelimit(key='post:username', rate='1/s'))
class Expensive():
    @rl 
    def dispatch():
    ...
@jsocol jsocol changed the title group name isn't correct group name isn't correct for CBVs Aug 7, 2015
@jsocol
Copy link
Owner

jsocol commented Aug 7, 2015

Gonna have to take a look at this. In the meantime, you can use the ratelimit mixin, especially if you're applying this to dispatch and not specific methods.

@jsocol jsocol added this to the 0.7 milestone Aug 7, 2015
@jplehmann
Copy link

I found this bug as well.

I have two class-based views and two method-based views. They are all set to group="main". The two CBV work together in the same group, and the MBV's do as well. But they do not count together (which would be explained by the above).

@jsocol
Copy link
Owner

jsocol commented Feb 21, 2019

So I've finally ended up coming across this and determined it's sort of (?) a bug in Django <2.1. Django 2.1 uses functools.partial which gives us access to the underlying method object to get the name and info from. Django 1.11 and 2.0 use this bound_func name which doesn't carry over the original attributes.

I have a fairly ugly solution, I think, that could definitely use testing and feedback as part of 3.0. It's part of #170.

@jsocol
Copy link
Owner

jsocol commented Feb 21, 2019

I have two class-based views and two method-based views. They are all set to group="main". The two CBV work together in the same group, and the MBV's do as well. But they do not count together (which would be explained by the above).

I apologize for not giving this a closer read before, but this doesn't sound like the same issue @jplehmann. The group value calculation only happens when it's not explicitly set, and that is what's failing in the case of @method_decorator(). The cache key also depends on the rate, the key/value, and the set of HTTP methods, so it's possible your issue stemmed from a difference in on of those?

@jsocol
Copy link
Owner

jsocol commented Feb 21, 2019

I believe this is fixed by #170 but definitely want feedback from folks testing it!

@jsocol jsocol closed this as completed Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants