Re: #14512: Decorating class based views (was Re: new class based views)
Taking my best guess I removed the attribute based method from my patch and uploaded a new one[1]. Would be good to know if there is something obviously broken about my patch. [1]: http://code.djangoproject.com/ticket/14512 Cheers, Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: #14512: Decorating class based views (was Re: new class based views)
Hi, I just added a patch to this ticket. It includes 2 solutions: First one is an implementation of Luke's idea with a class attribute. You just define an sequence: class MyView(View): decorators = [login_required, never_cache] # MyView.as_view === login_required(never_cache(View.as_view)) The `as_view` method then applies the decorators to the closure before returning it. This works nice with subclassing and isn't very magic. The important thing to note, is that all decorators are applied before the login enters dispatch(), so this is different then applying decorators to dispatch(). Second one, introduces a class decorator that decorates the as_view() method. This can be altered to use method_decorator() to decorate dispatch() instead, but during the whole process I decided that decorating as_view() works better (or maybe my brain turned into a cloud of steam from all this decoration). This approach has some serious pitfall, I wasn't aware of before: Subclassing in a decorator + super() in Python 2.x = trouble. See [1] and [2] for example of how to shot yourself in the foot with it. Thus, the decorator modifies the class it was given. I added a "subclass" argument, so you can write: MyView = view_decorator(login_required, subclass=True)(TemplateView) but it looks kinda clumsy. Comments here or directly on my github branch[3] very appreciated. Sorry, for another long email and thanks for reading it. [1]: http://github.com/lqc/django/commit/9b32817e00cdfa82568c45506e4c5b17cec68748#L0R53 [2]: http://gist.github.com/643536 [3]: http://github.com/lqc/django/commits/cbvdecoration_ticket14512 Best regards, Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: #14512: Decorating class based views (was Re: new class based views)
On 21 October 2010 16:53, Luke Plant wrote: > On Thu, 2010-10-21 at 15:02 +0200, Łukasz Rekucki wrote: > >> > Good catch! The attached patch should fix it, but it feels a bit icky, >> > due to the double application of the decorator. Let me know if it solves >> > this problem for you, or if you can think of a better way to fix it. >> >> I does fell icky. I haven't found a better way to solve this in >> general case, so I started thinking about a solution designed for CBV >> specifically. > > method_decorator does need fixing anyway, and the fix isn't as bad as I > first thought. The existing implementation already suffered from the > following issue: when you decorate a top-level function, the decorator > runs at the time the module is imported. But when you use the same > decorator with method_decorator on a method in a top-level class, the > decorator actually runs every time that method is called - *not* when > the module is imported and the class is defined. With my change, the > decorator also runs once when the class is defined, in addition to every > time the method is called. To me, both versions aren't obvious and definitly not something I would except from a decorator - to run only once. But I don't have any better ideas myself, so I won't complain. > >> Both are a variation on what you proposed earlier: >> >> 1) Add get_decorators() class method, that as is called by as_view(), >> to get a list of decorators >> to be applied on the closure. >> >> class MyView(View): >> @classmethod >> def get_decorators(cls): >> return (login_required,) >> >> This can be folded to a class decorator: >> >> def decorate_class(fdec): >> @wraps(fdec) >> def _decorator(cls): >> original_method = cls.get_decorators >> @wraps(original_method) >> def method_replacement(cls): >> return (fdec,) + original_method() >> cls.get_decorators = method_replacement >> return cls >> return _decorator >> >> @decorate_class(login_required): >> class MyView(View): >> pass > > This one seems much better than the decorators class property I > suggested - no hacking with MRO. Encouraging the use of the class > decorator seems like the way to go. However, there are some points with > your implementation: > > 1) It mutates the original class which is not a good idea really. I > should be able to do this: > > MyView1 = decorate_class(login_required)(MyView) > MyView2 = decorate_class(something_else)(MyView) > > and not have login_required applied to MyView to MyView2 > > That means subclassing inside decorate_class. I thought for a moment there, that modifing the original is inline with how function decorators work, but you're right. Do you consider subclassing inside a decorated bad, or is it ok? With subclassing, this is pretty much the same as converting a decorator to a Mixin: class MyView(Decorator(login_required), View): pass Of course there's an issuse with MRO and class identity. > > 2) We need to be careful about the order in which decorators are > applied. I would expect the following 3 chunks of code to be the > equivalent: > > @decorate_class(dec1) > @decorate_class(dec2) > class MyView(View): > # > > > class MyView(View): > def get_decorators(self): > return [dec1, dec2] > > > class MyView(View): > �...@method_decorator(dec1) > �...@method_decorator(dec2) > def dispatch(self, request, *args, **kwargs): > #... > > > All three should effectively be doing something like: > > view = dec1(dec2(view)) Agreed. 1 and 3 should be the same. 2 is actually broken. My original example should be: class MyView(View): @classmethod def get_decorators(cls): return (login_required,) + super(cls).get_decorators() That's a big -1 for this approach, 'cause it's error prone. If someone forgets the super() call it just resets the whole decorator stack (at least in my naive implementation). > > 3) And there is a general design issue. With this get_decorators() > method, there is both flexibility and some surprising behaviour: we > could write something like this: > > @decorate_class(dec1) > @decorate_class(dec2) > class MyView(View): > # > > > and someone else could write: > > class MySubClass(MyView): > �...@classmethod > def get_decorators(self): > return [] > > and dec1 and dec2 would not be applied. I don't know if this is a good > idea or not. If you consider decorators to be part of the > configurability that class based views are designed to support, then it > does make sense. If you consider them as a shortcut to defining the > intrinsic functionality of a view, it doesn't. Consider: @class_decorator(dec_x) class A(View): def dispatch(): #some code A @class_decorator(dec_y) class B(A) def dispatch(): #some code B What is the order of code execution: * (dec_y, codeB, dec_x, codeA) or * (dec_y, dec_x, codeB, codeA). I think it's the first optio
Re: #14512: Decorating class based views (was Re: new class based views)
On Thu, 2010-10-21 at 15:02 +0200, Łukasz Rekucki wrote: > > Good catch! The attached patch should fix it, but it feels a bit icky, > > due to the double application of the decorator. Let me know if it solves > > this problem for you, or if you can think of a better way to fix it. > > I does fell icky. I haven't found a better way to solve this in > general case, so I started thinking about a solution designed for CBV > specifically. method_decorator does need fixing anyway, and the fix isn't as bad as I first thought. The existing implementation already suffered from the following issue: when you decorate a top-level function, the decorator runs at the time the module is imported. But when you use the same decorator with method_decorator on a method in a top-level class, the decorator actually runs every time that method is called - *not* when the module is imported and the class is defined. With my change, the decorator also runs once when the class is defined, in addition to every time the method is called. > Both are a variation on what you proposed earlier: > > 1) Add get_decorators() class method, that as is called by as_view(), > to get a list of decorators > to be applied on the closure. > > class MyView(View): > @classmethod > def get_decorators(cls): > return (login_required,) > > This can be folded to a class decorator: > > def decorate_class(fdec): > @wraps(fdec) > def _decorator(cls): > original_method = cls.get_decorators > @wraps(original_method) > def method_replacement(cls): > return (fdec,) + original_method() > cls.get_decorators = method_replacement > return cls > return _decorator > > @decorate_class(login_required): > class MyView(View): > pass This one seems much better than the decorators class property I suggested - no hacking with MRO. Encouraging the use of the class decorator seems like the way to go. However, there are some points with your implementation: 1) It mutates the original class which is not a good idea really. I should be able to do this: MyView1 = decorate_class(login_required)(MyView) MyView2 = decorate_class(something_else)(MyView) and not have login_required applied to MyView to MyView2 That means subclassing inside decorate_class. 2) We need to be careful about the order in which decorators are applied. I would expect the following 3 chunks of code to be the equivalent: @decorate_class(dec1) @decorate_class(dec2) class MyView(View): # class MyView(View): def get_decorators(self): return [dec1, dec2] class MyView(View): @method_decorator(dec1) @method_decorator(dec2) def dispatch(self, request, *args, **kwargs): #... All three should effectively be doing something like: view = dec1(dec2(view)) 3) And there is a general design issue. With this get_decorators() method, there is both flexibility and some surprising behaviour: we could write something like this: @decorate_class(dec1) @decorate_class(dec2) class MyView(View): # and someone else could write: class MySubClass(MyView): @classmethod def get_decorators(self): return [] and dec1 and dec2 would not be applied. I don't know if this is a good idea or not. If you consider decorators to be part of the configurability that class based views are designed to support, then it does make sense. If you consider them as a shortcut to defining the intrinsic functionality of a view, it doesn't. Luke -- If you can't answer a man's arguments, all is not lost; you can still call him vile names. (Elbert Hubbard) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: #14512: Decorating class based views (was Re: new class based views)
On 21 October 2010 14:31, Luke Plant wrote: > On Thu, 2010-10-21 at 01:40 +0200, Łukasz Rekucki wrote: > >> On a related note, while writing a class_decorator() I noticed that >> method_decorator() doesn't work with decorators that set attributes on >> the view (like csrf_exempt). This is because the decorator is invoked >> on a closure inside _wrapper() during method call (i.e. during method >> call, not class creation), while as_view() only sees _wrapper - which >> has no attributes. > > Good catch! The attached patch should fix it, but it feels a bit icky, > due to the double application of the decorator. Let me know if it solves > this problem for you, or if you can think of a better way to fix it. I does fell icky. I haven't found a better way to solve this in general case, so I started thinking about a solution designed for CBV specifically. Both are a variation on what you proposed earlier: 1) Add get_decorators() class method, that as is called by as_view(), to get a list of decorators to be applied on the closure. class MyView(View): @classmethod def get_decorators(cls): return (login_required,) This can be folded to a class decorator: def decorate_class(fdec): @wraps(fdec) def _decorator(cls): original_method = cls.get_decorators @wraps(original_method) def method_replacement(cls): return (fdec,) + original_method() cls.get_decorators = method_replacement return cls return _decorator @decorate_class(login_required): class MyView(View): pass 2) decorators class property: class MyView(View): decorators = (login_required,) # in View: def as_view(cls, **initkwargs): # all the stuff now # now the hack part that follows MRO all_decorators = sum( (c.decorators for c in cls.mro() if "decorators" in c.__dict__), []) for dec in decorators: view = dec(view) return view This also can be turned into a class decorator. -- Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: #14512: Decorating class based views (was Re: new class based views)
On Thu, 2010-10-21 at 01:40 +0200, Łukasz Rekucki wrote:
> On a related note, while writing a class_decorator() I noticed that
> method_decorator() doesn't work with decorators that set attributes on
> the view (like csrf_exempt). This is because the decorator is invoked
> on a closure inside _wrapper() during method call (i.e. during method
> call, not class creation), while as_view() only sees _wrapper - which
> has no attributes.
Good catch! The attached patch should fix it, but it feels a bit icky,
due to the double application of the decorator. Let me know if it solves
this problem for you, or if you can think of a better way to fix it.
Luke
--
If you can't answer a man's arguments, all is not lost; you can
still call him vile names. (Elbert Hubbard)
Luke Plant || http://lukeplant.me.uk/
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
diff -r e0bdd0d406e9 django/utils/decorators.py
--- a/django/utils/decorators.py Thu Oct 21 06:01:02 2010 +
+++ b/django/utils/decorators.py Thu Oct 21 13:26:46 2010 +0100
@@ -11,15 +11,28 @@
"""
Converts a function decorator into a method decorator
"""
+# 'func' is a function at the time it is passed to _dec, but will eventually
+# be a method of the class it is defined it.
def _dec(func):
def _wrapper(self, *args, **kwargs):
+@decorator
def bound_func(*args2, **kwargs2):
return func(self, *args2, **kwargs2)
# bound_func has the signature that 'decorator' expects i.e. no
# 'self' argument, but it is a closure over self so it can call
# 'func' correctly.
-return decorator(bound_func)(*args, **kwargs)
-return wraps(func)(_wrapper)
+return bound_func(*args, **kwargs)
+# In case 'decorator' adds attributes to the function it decorates, we
+# want to copy those. We don't have access to bound_func in this scope,
+# but we can cheat by using it on a dummy function.
+@decorator
+def dummy(*args, **kwargs):
+pass
+update_wrapper(_wrapper, dummy)
+# Need to preserve any existing attributes of 'func', including the name.
+update_wrapper(_wrapper, func)
+
+return _wrapper
update_wrapper(_dec, decorator)
# Change the name to aid debugging.
_dec.__name__ = 'method_decorator(%s)' % decorator.__name__
diff -r e0bdd0d406e9 tests/regressiontests/decorators/tests.py
--- a/tests/regressiontests/decorators/tests.py Thu Oct 21 06:01:02 2010 +
+++ b/tests/regressiontests/decorators/tests.py Thu Oct 21 13:26:46 2010 +0100
@@ -129,14 +129,58 @@
simple_dec_m = method_decorator(simple_dec)
+# For testing method_decorator, two decorators that add an attribute to the function
+def myattr_dec(func):
+def wrapper(*args, **kwargs):
+return func(*args, **kwargs)
+wrapper.myattr = True
+return wraps(func)(wrapper)
+
+myattr_dec_m = method_decorator(myattr_dec)
+
+
+def myattr2_dec(func):
+def wrapper(*args, **kwargs):
+return func(*args, **kwargs)
+wrapper.myattr2 = True
+return wraps(func)(wrapper)
+
+myattr2_dec_m = method_decorator(myattr2_dec)
+
+
class MethodDecoratorTests(TestCase):
"""
Tests for method_decorator
"""
-def test_method_decorator(self):
+def test_preserve_signature(self):
class Test(object):
@simple_dec_m
def say(self, arg):
return arg
self.assertEqual("test:hello", Test().say("hello"))
+
+def test_preserve_attributes(self):
+# Sanity check myattr_dec and myattr2_dec
+@myattr_dec
+@myattr2_dec
+def foo():
+pass
+
+self.assertEqual(getattr(foo, 'myattr', False), True)
+self.assertEqual(getattr(foo, 'myattr2', False), True)
+
+# Now check method_decorator
+class Test(object):
+@myattr_dec_m
+@myattr2_dec_m
+def method(self):
+pass
+
+self.assertEqual(getattr(Test().method, 'myattr', False), True)
+self.assertEqual(getattr(Test().method, 'myattr2', False), True)
+
+self.assertEqual(getattr(Test.method, 'myattr', False), True)
+self.assertEqual(getattr(Test.method, 'myattr2', False), True)
+
+self.assertEqual(Test.method.__func__.__name__, 'method')
Re: new class based views
On Thu, Oct 21, 2010 at 1:42 AM, Luke Plant wrote: > On Wed, 2010-10-20 at 16:05 +0200, Łukasz Rekucki wrote: > >> OTOH, it's annoying to have to write an dispatch() method with a super >> inside (which effectively does nothing), just to apply a decorator. > > That's a good point I hadn't thought of, as are Russell's other points. > > Just to put it on the table, another option we thought of at one point > was to have a 'decorators' attribute on the class, which is a list of > decorators that is applied by the as_view() method. It would in theory > allow you to add decorators to either end of the set of decorators that > are applied, something like this: > > class MyView(SomeOtherView): > > decorators = SomeOtherView.decorators + [my_dec_1, my_dec_2] > > It gets a bit trickier with multiple inheritance. I don't know if that > would be too much of a big deal - I would imagine that most of the mixin > classes would not be providing decorators. But then my imagination is > probably limited. Meh - this seems like reinventing a syntactic wheel to me. Python 2.6 has class decorators. They work in a predictable way, with predictable MRO combination rules. Trying to invent a new class-attribute syntax for class decoration seems like asking for trouble. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
#14512: Decorating class based views (was Re: new class based views)
On 20 October 2010 19:42, Luke Plant wrote: > On Wed, 2010-10-20 at 16:05 +0200, Łukasz Rekucki wrote: > >> OTOH, it's annoying to have to write an dispatch() method with a super >> inside (which effectively does nothing), just to apply a decorator. > > That's a good point I hadn't thought of, as are Russell's other points. > > Just to put it on the table, another option we thought of at one point > was to have a 'decorators' attribute on the class, which is a list of > decorators that is applied by the as_view() method. It would in theory > allow you to add decorators to either end of the set of decorators that > are applied, something like this: > > class MyView(SomeOtherView): > > decorators = SomeOtherView.decorators + [my_dec_1, my_dec_2] I'm addicted to decorators so I would probably end up with a decorator that adds items to that list :) > > It gets a bit trickier with multiple inheritance. I don't know if that > would be too much of a big deal - I would imagine that most of the mixin > classes would not be providing decorators. But then my imagination is > probably limited. I didn't find a use case for decorating a mixin yet, but i'm only starting to explore (I didn't have this problem in any of my own applications that use CBV). There is a simple pattern to change a decorator into a mixin that should play well with multiple inheritance. On a related note, while writing a class_decorator() I noticed that method_decorator() doesn't work with decorators that set attributes on the view (like csrf_exempt). This is because the decorator is invoked on a closure inside _wrapper() during method call (i.e. during method call, not class creation), while as_view() only sees _wrapper - which has no attributes. -- Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On Wed, 2010-10-20 at 16:05 +0200, Łukasz Rekucki wrote: > OTOH, it's annoying to have to write an dispatch() method with a super > inside (which effectively does nothing), just to apply a decorator. That's a good point I hadn't thought of, as are Russell's other points. Just to put it on the table, another option we thought of at one point was to have a 'decorators' attribute on the class, which is a list of decorators that is applied by the as_view() method. It would in theory allow you to add decorators to either end of the set of decorators that are applied, something like this: class MyView(SomeOtherView): decorators = SomeOtherView.decorators + [my_dec_1, my_dec_2] It gets a bit trickier with multiple inheritance. I don't know if that would be too much of a big deal - I would imagine that most of the mixin classes would not be providing decorators. But then my imagination is probably limited. Luke -- If you can't answer a man's arguments, all is not lost; you can still call him vile names. (Elbert Hubbard) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
Created a ticket for this: http://code.djangoproject.com/ticket/14512 On 20 October 2010 16:13, Russell Keith-Magee wrote: > On Wed, Oct 20, 2010 at 9:59 PM, Luke Plant wrote: >> On Wed, 2010-10-20 at 19:25 +0800, Russell Keith-Magee wrote: >> >>> A class decorator (and/or a wrapper function for turning decorators >>> into class decorators) is also an interesting idea. >> >> An argument against this is "one way to do it". If I look at a class >> and want to know what decorators are applied, if would be nice if there >> was only one place to look. I guess that that one place could be the >> class rather than the 'dispatch' method, if we encourage that practice, >> but that pattern might be hard to encourage since class-decoration is >> less obvious (and certainly much less well established in the Django >> code base) than method decoration. > > I'm not sure that argument holds up. Right now, we already have 2 ways > to do it - you can decorate the view function at time of definition, > and you can decorate when it is included in the URL pattern. > > The class-based view case is also complicated by the common usage > pattern. Consider a prototypical view: > > class MyView(View): > def get(self, request, *args, **kwargs): > ... > def post(self, request, *args, **kwargs): > ... > > You don't have to define dispatch or as_view -- the two places that > can be decorated to provide class-wide behavior. You could decorate > both get *and* post -- which may be a good approach in some cases (say > you want all GETs to be allowed, but POSTs to be login protected) -- > but it's an unusual duplication to require for the common case. You > could also decorate the MyView.as_view() line in the URL pattern, but > that doesn't enforce the decorator for every use. > > There's no simple direct analog of the "make sure every access to this > view is decorated". To me, a class-view-decorator seems like a way to > express the existing use case. > > Yours, > Russ Magee %-) > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On Wed, Oct 20, 2010 at 9:59 PM, Luke Plant wrote: > On Wed, 2010-10-20 at 19:25 +0800, Russell Keith-Magee wrote: > >> A class decorator (and/or a wrapper function for turning decorators >> into class decorators) is also an interesting idea. > > An argument against this is "one way to do it". If I look at a class > and want to know what decorators are applied, if would be nice if there > was only one place to look. I guess that that one place could be the > class rather than the 'dispatch' method, if we encourage that practice, > but that pattern might be hard to encourage since class-decoration is > less obvious (and certainly much less well established in the Django > code base) than method decoration. I'm not sure that argument holds up. Right now, we already have 2 ways to do it - you can decorate the view function at time of definition, and you can decorate when it is included in the URL pattern. The class-based view case is also complicated by the common usage pattern. Consider a prototypical view: class MyView(View): def get(self, request, *args, **kwargs): ... def post(self, request, *args, **kwargs): ... You don't have to define dispatch or as_view -- the two places that can be decorated to provide class-wide behavior. You could decorate both get *and* post -- which may be a good approach in some cases (say you want all GETs to be allowed, but POSTs to be login protected) -- but it's an unusual duplication to require for the common case. You could also decorate the MyView.as_view() line in the URL pattern, but that doesn't enforce the decorator for every use. There's no simple direct analog of the "make sure every access to this view is decorated". To me, a class-view-decorator seems like a way to express the existing use case. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
Well, since we are already using class Mixins for extending View functionality, we can try to turn decorators into mixins. Having two ways to add some functionality to class (decorators and mixins) can be somewhat uncomfortable. I had some doubts if mixins can act as a "wrappers" to defined functions, but, as I was told, these doubts were based on my poor understanding of how "super()" and mixins work. I totally agree with Łukasz about annoyance of decorating a dispatch() function. -- Best Regards, Valentin Golev Lead Developer r00, http://r00.ru http://valyagolev.net +7 921 789 0895, avaiable 12:00-18:00 MSK On Wed, Oct 20, 2010 at 5:59 PM, Luke Plant wrote: > On Wed, 2010-10-20 at 19:25 +0800, Russell Keith-Magee wrote: > >> A class decorator (and/or a wrapper function for turning decorators >> into class decorators) is also an interesting idea. > > An argument against this is "one way to do it". If I look at a class > and want to know what decorators are applied, if would be nice if there > was only one place to look. I guess that that one place could be the > class rather than the 'dispatch' method, if we encourage that practice, > but that pattern might be hard to encourage since class-decoration is > less obvious (and certainly much less well established in the Django > code base) than method decoration. > > Luke > > -- > If you can't answer a man's arguments, all is not lost; you can > still call him vile names. (Elbert Hubbard) > > Luke Plant || http://lukeplant.me.uk/ > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On 20 October 2010 15:59, Luke Plant wrote: > On Wed, 2010-10-20 at 19:25 +0800, Russell Keith-Magee wrote: > >> A class decorator (and/or a wrapper function for turning decorators >> into class decorators) is also an interesting idea. > > An argument against this is "one way to do it". If I look at a class > and want to know what decorators are applied, if would be nice if there > was only one place to look. I guess that that one place could be the > class rather than the 'dispatch' method, if we encourage that practice, > but that pattern might be hard to encourage since class-decoration is > less obvious (and certainly much less well established in the Django > code base) than method decoration. OTOH, it'a annoying to have to write an dispatch() method with a super inside (which effectively does nothing), just to apply a decorator. -- Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On Wed, 2010-10-20 at 19:25 +0800, Russell Keith-Magee wrote: > A class decorator (and/or a wrapper function for turning decorators > into class decorators) is also an interesting idea. An argument against this is "one way to do it". If I look at a class and want to know what decorators are applied, if would be nice if there was only one place to look. I guess that that one place could be the class rather than the 'dispatch' method, if we encourage that practice, but that pattern might be hard to encourage since class-decoration is less obvious (and certainly much less well established in the Django code base) than method decoration. Luke -- If you can't answer a man's arguments, all is not lost; you can still call him vile names. (Elbert Hubbard) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
I think class decorator for views only is a great idea, because it will be: a) just like with old view functions decorators b) does not require any method overrides/imports into urls.py/explicit transformations in views.py I'd like Django to have a decorator for turning old decorators into decorators for class based views. I've hardly seen @login_required and lots of other common decorators not used in the view context, so why avoid being attached to Django views interface, since we already made up our minds about having such an interface. So I think, both tickets - for docs and for "@on_dispatch()" decorators - are worth it. -- Best Regards, Valentin Golev Lead Developer r00, http://r00.ru http://valyagolev.net +7 921 789 0895, avaiable 12:00-18:00 MSK On Wed, Oct 20, 2010 at 3:25 PM, Russell Keith-Magee wrote: > On Wed, Oct 20, 2010 at 7:05 PM, Valentin Golev wrote: >> As far as I know, I can just say: >> >> @login_required >> class MyView(View): >> # ... >> >> since it will wrap __init__ >> >> I need either to create a decorator for decorators: >> @on_dispatch(login_required) >> class MyView(View): >> >> or decorate result of MyView().as_view(). >> >> Maybe I'm wrong. I think it will be great to add something about it to >> docs, since decorators is extremely wide-used feature. > > As I've commented on the django-users thread, Django already provides > method_decorator(), which provides a way to decorate the dispatch > function (or any individual get/post etc HTTP method) on a class-based > view. > > The other simple way to decorate a view is to decorate the result of > the as_view() call in the URL pattern -- i.e., instead of deploying: > > MyView.as_view() > > deploy: > > login_required(MyView.as_view()) > > A class decorator (and/or a wrapper function for turning decorators > into class decorators) is also an interesting idea. However, unlike > method_decorator(), a class_decorator() would need to be > view-specific, since it needs to modify the class it wraps in a > specific way, rather than the completely generic approach that > method_decorator() can take. > > Regardless, your point about documenting these approaches is well > taken. It isn't immediately obvious how decoration should work in a > class-based world. If you open a ticket for this, that will help make > sure that this idea isn't forgotten. > > Yours, > Russ Magee %-) > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On Wed, Oct 20, 2010 at 7:05 PM, Valentin Golev wrote: > As far as I know, I can just say: > > @login_required > class MyView(View): > # ... > > since it will wrap __init__ > > I need either to create a decorator for decorators: > @on_dispatch(login_required) > class MyView(View): > > or decorate result of MyView().as_view(). > > Maybe I'm wrong. I think it will be great to add something about it to > docs, since decorators is extremely wide-used feature. As I've commented on the django-users thread, Django already provides method_decorator(), which provides a way to decorate the dispatch function (or any individual get/post etc HTTP method) on a class-based view. The other simple way to decorate a view is to decorate the result of the as_view() call in the URL pattern -- i.e., instead of deploying: MyView.as_view() deploy: login_required(MyView.as_view()) A class decorator (and/or a wrapper function for turning decorators into class decorators) is also an interesting idea. However, unlike method_decorator(), a class_decorator() would need to be view-specific, since it needs to modify the class it wraps in a specific way, rather than the completely generic approach that method_decorator() can take. Regardless, your point about documenting these approaches is well taken. It isn't immediately obvious how decoration should work in a class-based world. If you open a ticket for this, that will help make sure that this idea isn't forgotten. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
Almost everything I'm talking about right now is from this thread:
http://groups.google.com/group/django-users/browse_frm/thread/5239e284b5c285d5/a2676f257d37cf85#a2676f257d37cf85
("login_required and new class based views" in django-users in case
the link doesnt work.)
--
Best Regards,
Valentin Golev
Lead Developer
r00, http://r00.ru
http://valyagolev.net
+7 921 789 0895, avaiable 12:00-18:00 MSK
On Wed, Oct 20, 2010 at 3:05 PM, Valentin Golev wrote:
> As far as I know, I can just say:
>
> @login_required
> class MyView(View):
> # ...
>
> since it will wrap __init__
>
> I need either to create a decorator for decorators:
> @on_dispatch(login_required)
> class MyView(View):
>
> or decorate result of MyView().as_view().
>
> Maybe I'm wrong. I think it will be great to add something about it to
> docs, since decorators is extremely wide-used feature.
>
> --
> Best Regards,
> Valentin Golev
> Lead Developer
> r00, http://r00.ru
>
> http://valyagolev.net
> +7 921 789 0895, avaiable 12:00-18:00 MSK
>
>
>
> On Wed, Oct 20, 2010 at 2:59 PM, Russell Keith-Magee
> wrote:
>> On Wed, Oct 20, 2010 at 6:01 PM, Valentin Golev wrote:
>>> Awesome, thank you!
>>>
>>> I've asked about @login_required and class based views in
>>> django-users, and I'd like to ask here: are something like
>>> LoginRequiredMixin's planned?
>>
>> No, nothing like that is planned. This is a situation were decorators
>> will be a better match in the general case, because login redirection
>> can usually be transparently wrapped around a view.
>>
>> You should be able to use decorators on class based views in the same
>> way that you use them on normal function views.
>>
>> Yours,
>> Russ Magee %-)
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers" group.
>> To post to this group, send email to django-develop...@googlegroups.com.
>> To unsubscribe from this group, send email to
>> django-developers+unsubscr...@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/django-developers?hl=en.
>>
>>
>
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
As far as I know, I can just say: @login_required class MyView(View): # ... since it will wrap __init__ I need either to create a decorator for decorators: @on_dispatch(login_required) class MyView(View): or decorate result of MyView().as_view(). Maybe I'm wrong. I think it will be great to add something about it to docs, since decorators is extremely wide-used feature. -- Best Regards, Valentin Golev Lead Developer r00, http://r00.ru http://valyagolev.net +7 921 789 0895, avaiable 12:00-18:00 MSK On Wed, Oct 20, 2010 at 2:59 PM, Russell Keith-Magee wrote: > On Wed, Oct 20, 2010 at 6:01 PM, Valentin Golev wrote: >> Awesome, thank you! >> >> I've asked about @login_required and class based views in >> django-users, and I'd like to ask here: are something like >> LoginRequiredMixin's planned? > > No, nothing like that is planned. This is a situation were decorators > will be a better match in the general case, because login redirection > can usually be transparently wrapped around a view. > > You should be able to use decorators on class based views in the same > way that you use them on normal function views. > > Yours, > Russ Magee %-) > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On Wed, Oct 20, 2010 at 6:01 PM, Valentin Golev wrote: > Awesome, thank you! > > I've asked about @login_required and class based views in > django-users, and I'd like to ask here: are something like > LoginRequiredMixin's planned? No, nothing like that is planned. This is a situation were decorators will be a better match in the general case, because login redirection can usually be transparently wrapped around a view. You should be able to use decorators on class based views in the same way that you use them on normal function views. Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
Awesome, thank you! I've asked about @login_required and class based views in django-users, and I'd like to ask here: are something like LoginRequiredMixin's planned? -- Best Regards, Valentin Golev Lead Developer r00, http://r00.ru http://valyagolev.net +7 921 789 0895, avaiable 12:00-18:00 MSK On Wed, Oct 20, 2010 at 4:36 AM, Russell Keith-Magee wrote: > On Wed, Oct 20, 2010 at 1:25 AM, Valentin Golev wrote: >> Hello, >> >> I'm trying to use brand new Class Based Views and I have a question >> about implementation. >> >> Let's take a look at SingleObjectMixin.get_object(): >> http://code.djangoproject.com/browser/django/trunk/django/views/generic/detail.py >> >> Why does it use function arguments, and not self.kwargs, while, for >> example, View.get() or post() methods use instance variables? >> >> Is it going to change? I'm just curious. I understand that since this >> is not the release, API is subject to change, so I'd like to know is >> it going to change and how. > > Good catch! I've just made a change (r14292) to make get_object() more > consistent with the rest of the class-based view API, dropping the > (pk=None, slug=None, **kwargs) arguments in favor of using > self.kwargs. I've also added formal API documentation for > get_object(), which was omitted for some reason. > > Thanks for the feedback! > > Yours, > Russ Magee %-) > > -- > You received this message because you are subscribed to the Google Groups > "Django developers" group. > To post to this group, send email to django-develop...@googlegroups.com. > To unsubscribe from this group, send email to > django-developers+unsubscr...@googlegroups.com. > For more options, visit this group at > http://groups.google.com/group/django-developers?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: new class based views
On Wed, Oct 20, 2010 at 1:25 AM, Valentin Golev wrote: > Hello, > > I'm trying to use brand new Class Based Views and I have a question > about implementation. > > Let's take a look at SingleObjectMixin.get_object(): > http://code.djangoproject.com/browser/django/trunk/django/views/generic/detail.py > > Why does it use function arguments, and not self.kwargs, while, for > example, View.get() or post() methods use instance variables? > > Is it going to change? I'm just curious. I understand that since this > is not the release, API is subject to change, so I'd like to know is > it going to change and how. Good catch! I've just made a change (r14292) to make get_object() more consistent with the rest of the class-based view API, dropping the (pk=None, slug=None, **kwargs) arguments in favor of using self.kwargs. I've also added formal API documentation for get_object(), which was omitted for some reason. Thanks for the feedback! Yours, Russ Magee %-) -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Re: ,new class based views
Sorry! I've hit "Enter" key accidentally while writing a subject. Don't blame me too much, please :) On 19 окт, 21:22, Valentin Golev wrote: > Hello -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-develop...@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
