Re: RFC: "universal" view decorators

2013-06-08 Thread Donald Stufft

On Jun 8, 2013, at 12:48 PM, Hanne Moa  wrote:

> On 3 June 2013 18:09,  wrote:
> # `DecoratorMixin`
> 
> `DecoratorMixin` is a class factory that converts any view decorator into a 
> class-based view mixin. 
> 
> Using it is easy and intuitive:
> 
> LoginRequiredMixin = DecoratorMixin(login_required)
> 
> Now this, this feels pythonic. Bravo!  I'm so doing this in future 
> projects... (Though, class factories are spelled lowercase_with_underscores, 
> no? At least in Django. For instance formset_factory().)
> 
> I've had a look at various LoginRequiredMixins out there (everybody makes 
> their own it seems, including me). It is quite common that it doesn't cover 
> all the things that login_required covers (like supporting the new way of 
> doing users in 1.5) so in the case of  LoginRequiredMixin, I think the safest 
> solution is to include a mixin in core anyway.
> 
> 
> HM
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers?hl=en.
> For more options, visit https://groups.google.com/groups/opt_out.
>  
>  

Part of the reasoning of my original Change to make decorators classes was that 
it enabled much easier customization of them. Currently you basically either 
hope there was an option for doing what you wanted, or you copy/paste the 
entire thing and modify it.

Using a class lets you override it.

-
Donald Stufft
PGP: 0x6E3CBCE93372DCFA // 7C6B 7C5D 5E2B 6356 A926 F04F 6E3C BCE9 3372 DCFA



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: RFC: "universal" view decorators

2013-06-08 Thread Hanne Moa
On 3 June 2013 18:09,  wrote:

> # `DecoratorMixin`
>
> `DecoratorMixin` is a class factory that converts any view decorator into
> a class-based view mixin.
>
> Using it is easy and intuitive:
>
> LoginRequiredMixin = DecoratorMixin(login_required)
>

Now this, this feels pythonic. Bravo!  I'm so doing this in future
projects... (Though, class factories are spelled
lowercase_with_underscores, no? At least in Django. For instance
formset_factory().)

I've had a look at various LoginRequiredMixins out there (everybody makes
their own it seems, including me). It is quite common that it doesn't cover
all the things that login_required covers (like supporting the new way of
doing users in 1.5) so in the case of  LoginRequiredMixin, I think the
safest solution is to include a mixin in core anyway.


HM

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: RFC: "universal" view decorators

2013-06-03 Thread gavinwahl
Attempting to make decorators anticipate every usage is the wrong approach. 
In Django, the sole interface to a view is as a callable that accepts an 
`HttpRequest` and turns it into an `HttpResponse` or exception. Django the 
framework doesn't actually need to support class-based views--because CBVs 
are just another way to build view callables.

View callables are an elegant abstraction that keeps the core of Django, 
and the decorators it provides, simple. Trying to anticipate all the ways 
of building view callables is not only complex, it is impossible. Why 
shouldn't I be able to use the built-in decorators with my own 
implementations of class-based views? If the built-in decorators' 
implementations are simple, I will be able to provide an adapter to use 
them with my CBV system. There are many useful decorators built-in to 
Django and in the wild, and my adapter will work with all of them, because 
they treat views as simple callables.

`method_decorator` is the right idea, but it is painful to use. It requires 
a tedious empty `dispatch` method, just to have something to decorate. 
Using decorators to modify views makes sense, because decoration is the 
only way to modify the functionality of a callable. Classes, however, have 
much more powerful ways of adding optional functionality: multiple 
inheritance.


# `DecoratorMixin`

`DecoratorMixin` is a class factory that converts any view decorator into a 
class-based view mixin. It works as you would expect with inheritance--MRO 
provides the same ordering that function decorators do, and multiple 
inheritance allows the mixins to be composed naturally. Inheriting from a 
decorated view decorates the subclass as you would expect. It averts the 
need for the annoying empty `dispatch` method in each class that wants to 
use a decorator.

Its implementation is simple: https://gist.github.com/gavinwahl/5694349. 
This is a complete implementation, and is all that's necessary to use view 
decorators with Django's CBVs. I use this code in all my projects with 
great success. [Here is an 
example](https://github.com/fusionbox/django-authtools/blob/master/authtools/views.py#L94)
 
of its use in a reimplementation of Django's auth views as CBVs.

Using it is easy and intuitive:

LoginRequiredMixin = DecoratorMixin(login_required)
 
class LoginRequiredAndCsrfMixin(DecoratorMixin(login_required),
DecoratorMixin(csrf_protect)):
"""
Inheriting from this class is the same as decorating your view 
thus::

@login_required
@csrf_protect
def someview(request):
pass
"""
pass

class SomeView(LoginRequiredMixin, View):
# ...
   
   
I propose `DecoratorMixin` be added to Django. It retains compatibility 
with all existing decorators, elegantly adapts functional decorators to 
good object-oriented practices, and has a simple implementation. 
'Universal' decorators don't accommodate existing third-party code, contain 
complex magic, and annul one of the most elegant principles of Django: 
views are simply callables.


On Thursday, September 15, 2011 2:44:39 PM UTC-6, Jacob Kaplan-Moss wrote:
>
> Hi folks --
>
> I'd like to convert all the view decorators built into Django to be
> "universal" -- so they'll work to decorate *any* view, whether a
> function, method, or class. I believe I've figured out a technique for
> this, but I'd like some feedback on the approach before I dive in too
> deep.
>
> Right now view decorators (e.g. @login_required) only work on
> functions. If you want to use a decorator on a method then you need to
> "convert" the decorator using method_decorator(original_decorator).
> You can't use view decorators on class-based views at all. This means
> making a class-based view require login requires this awesomeness::
>
> class MyView(View):
> @method_decorator(login_required)
> def dispatch(self, *args, **kwargs):
> return super(MyView, self.dispatch(*args, **kwargs)
>
> This makes me sad. It's really counter-intuitive and relies on a
> recognizing that functions and methods are different to even know to
> look for method_decorator.
>
> #14512 proposes a adding another view-decorator-factory for decorating
> class-based views, which would turn the above into::
>
> @class_view_decorator(login_required)
> class MyView(View):
> ...
>
> This makes me less sad, but still sad. Factory functions. Ugh.
>
> I want @login_required to work for anything::
>
> @login_required
> class MyView(View):
> ...
>
> class Something(object):
> @login_required
> def my_view(self, request):
> ...
>
> @login_required
> def my_view(request):
> ...
>
>
> Now, back in the day we somewhat had this: decorators tried to work
> with both functions and methods. The technique turned out not to work
> 

Re: RFC: "universal" view decorators

2013-05-21 Thread ptone


On Saturday, May 18, 2013 5:57:49 AM UTC-7, Marc Tamlyn wrote:
>
> I'm going to resurrect this old thread as I'd like to get it resolved in 
> some fashion.
>
> I used to be very in favour of the class decorators approach. I've been 
> using an implementation of `@class_view_decorator(func)` for some time and 
> it works pretty well. That said my implementation at least was subject to a 
> notable flaw which is that if multiple parts of the hierarchy have the same 
> decorator applied to them then the check gets done twice. Mixins are much 
> cleverer than this because of MRO, so we avoid that problem.
>
> Mixins however have their own issues - it's "harder" (for some definition) 
> to ensure that all of your top-level permission checking happening in the 
> correct order. That said, I am certainly veering towards implementing this 
> using mixins (for each piece of functionality).
>
> I'd really like to have a look at Donald's implementation, sadly it seems 
> to no longer be on github. Do you still have then code somewhere, or can 
> you explain the implementation idea?
>

Marc, ask and you shall receive.

I've always been convinced this is the way to go:

https://github.com/ptone/django/compare/mixin-decorators

-P
 

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.




Re: RFC: "universal" view decorators

2013-05-18 Thread Donald Stufft

On May 18, 2013, at 8:57 AM, Marc Tamlyn  wrote:

> I'm going to resurrect this old thread as I'd like to get it resolved in some 
> fashion.
> 
> I used to be very in favour of the class decorators approach. I've been using 
> an implementation of `@class_view_decorator(func)` for some time and it works 
> pretty well. That said my implementation at least was subject to a notable 
> flaw which is that if multiple parts of the hierarchy have the same decorator 
> applied to them then the check gets done twice. Mixins are much cleverer than 
> this because of MRO, so we avoid that problem.
> 
> Mixins however have their own issues - it's "harder" (for some definition) to 
> ensure that all of your top-level permission checking happening in the 
> correct order. That said, I am certainly veering towards implementing this 
> using mixins (for each piece of functionality).
> 
> I'd really like to have a look at Donald's implementation, sadly it seems to 
> no longer be on github. Do you still have then code somewhere, or can you 
> explain the implementation idea?

I'm sure I have it somewhere, but knowing where is another thing all together ;)

If I recall I made a mixin superclass that added an ``as_decorator()`` class 
function which functioned similarly to the ``as_view()`` function. It took a 
common entry point and generated a non class function based on that and 
returned it.

As far as the actual mixins themselves I'd take a look at django-braces which 
has implementations of a lot of the decorators as mixins already they'd just 
need maybe some tweaking and then code added to allow turning them into actual 
decorators.

> 
> Marc
> 
> 
> On Thursday, 15 September 2011 22:44:39 UTC+2, Jacob Kaplan-Moss wrote:
> Hi folks --
> I'd like to convert all the view decorators built into Django to be
> "universal" -- so they'll work to decorate *any* view, whether a
> function, method, or class. I believe I've figured out a technique for
> this, but I'd like some feedback on the approach before I dive in too
> deep.
> 
> Right now view decorators (e.g. @login_required) only work on
> functions. If you want to use a decorator on a method then you need to
> "convert" the decorator using method_decorator(original_decorator).
> You can't use view decorators on class-based views at all. This means
> making a class-based view require login requires this awesomeness::
> 
> class MyView(View):
> @method_decorator(login_required)
> def dispatch(self, *args, **kwargs):
> return super(MyView, self.dispatch(*args, **kwargs)
> 
> This makes me sad. It's really counter-intuitive and relies on a
> recognizing that functions and methods are different to even know to
> look for method_decorator.
> 
> #14512 proposes a adding another view-decorator-factory for decorating
> class-based views, which would turn the above into::
> 
> @class_view_decorator(login_required)
> class MyView(View):
> ...
> 
> This makes me less sad, but still sad. Factory functions. Ugh.
> 
> I want @login_required to work for anything::
> 
> @login_required
> class MyView(View):
> ...
> 
> class Something(object):
> @login_required
> def my_view(self, request):
> ...
> 
> @login_required
> def my_view(request):
> ...
> 
> 
> Now, back in the day we somewhat had this: decorators tried to work
> with both functions and methods. The technique turned out not to work
> (see #12804) and was removed in [12399]. See
> http://groups.google.com/group/django-developers/browse_thread/thread/3024b14a47f5404d
> for the discussion.
> 
> I believe, however, I've figured out a different technique to make
> this work: don't try to detect bound versus unbound methods, but
> instead look for the HttpRequest object. It'll either be args[0] if
> the view's a function, or args[1] if the view's a method. This
> technique won't work for any old decorator, but it *will* work (I
> think) for any decorator *designed to be applied only to views*.
> 
> I've written a proof-of-concept patch to @login_required (well,
> @user_passes_test, actually):
> 
> https://gist.github.com/1220375
> 
> The test suite passes with this, with one exception:
> https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/decorators/tests.py#L87.
> I maintain that this test is broken and should be using RequestFactory
> instead.
> 
> Can I get some thoughts on this technique and some feedback on whether
> it's OK to apply to every decorator built into Django?
> 
> Jacob
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit 

Re: RFC: "universal" view decorators

2011-10-12 Thread Andre Terra
Definitely +1 on this idea. It seems to me like the cleanest solution yet
proposed insofar as it provides the desired new functionality while
requiring very little change for current code.


Cheers,
AT

On Wed, Oct 12, 2011 at 5:31 PM, Donald Stufft wrote:

> Sorry for the delay,
>
> Here's a possible method of handling the idea of adding the additional
> functionality of things like requiring logins to both FBV's and CBV's in a
> way that I feel is consistent with the methodologies of each of those types
> (decorators for FBV, Mixins for CBV).
>
> This also keeps all the logic in one place. It allows for anyone (even
> people using FBV's) to easily extend the decorators and create their own
> versions of them, without having to rewrite the whole thing (which was one
> of the major reasons I like CBV's). It shouldn't break anything, the tests
> are passing and a functional test in a browser they all appear to be
> working.
>
> https://github.com/dstufft/django/compare/master...mixin-decorators
>
> Thoughts? Good Idea? Bad Idea?
>
> On Thursday, September 22, 2011 at 2:49 AM, ptone wrote:
>
>
>
> On Sep 21, 8:44 am, Donald Stufft  wrote:
>
>
>
> The goal is to provide a Mixin for class based views, a decorator for class
> based views, a decorator for methods, and a decorator for functions, all
> from the same codebase.
>
> Currently I have the decorator for classes working, and the mixin working.
> I have the places where the other 2 will go but from a combination of not
> being very good at decorators, and not feeling well I haven't finished it
> yet.
>
>
> I like the overall direction of this approach (mixin class as the root
> container of functionality) - but a number of technical issues still
> need to have a resolution demonstrated (ie the decorating function
> behavior)
>
> One, perhaps pony, of a universal approach, would be to convert the
> decorator kwargs into class attributes, so that they could be
> overriden in subclasses of a decorated parent, something easy to do
> with mixins as the starting point.
>
> Also on a somewhat related note, a better option for middleware
> process_view to interact with class based views
>
> -Preston
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@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-developers@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-developers@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: RFC: "universal" view decorators

2011-09-22 Thread ptone


On Sep 21, 8:44 am, Donald Stufft  wrote:


>
> The goal is to provide a Mixin for class based views, a decorator for class 
> based views, a decorator for methods, and a decorator for functions, all from 
> the same codebase.
>
> Currently I have the decorator for classes working, and the mixin working. I 
> have the places where the other 2 will go but from a combination of not being 
> very good at decorators, and not feeling well I haven't finished it yet.

I like the overall direction of this approach (mixin class as the root
container of functionality) - but a number of technical issues still
need to have a resolution demonstrated (ie the decorating function
behavior)

One, perhaps pony, of a universal approach, would be to convert the
decorator kwargs into class attributes, so that they could be
overriden in subclasses of a decorated parent, something easy to do
with mixins as the starting point.

Also on a somewhat related note, a better option for middleware
process_view to interact with class based views

-Preston

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-21 Thread Donald Stufft
I just got my wisdom teeth removed so i'm not exactly feeling the greatest, 
however I committed what I've done so far and pushed to my copy of django on 
github.

The goal is to provide a Mixin for class based views, a decorator for class 
based views, a decorator for methods, and a decorator for functions, all from 
the same codebase.

Currently I have the decorator for classes working, and the mixin working. I 
have the places where the other 2 will go but from a combination of not being 
very good at decorators, and not feeling well I haven't finished it yet.

Anyways here's what I've gotten so far: 
https://github.com/dstufft/django/compare/master...universal-mixin-decorators 


On Wednesday, September 21, 2011 at 8:26 AM, Roald de Vries wrote:

> Hi all,
> 
> Haven't seen a comment on this topic for a few days, so was wondering if a 
> core dev can make a design decision yet. I have some spare time in the 
> remainder of this week, so if the (universal) decorator-approach is chosen, I 
> should be able to finish it shortly (the mixin-approach is a bit harder to 
> estimate).
> 
> Cheers, Roald
> 
> 
> On Sep 17, 2011, at 4:32 AM, Alex Gaynor wrote:
> > On Fri, Sep 16, 2011 at 10:27 PM, Donald Stufft  > (mailto:donald.stu...@gmail.com)> wrote:
> > > unittest.skip isn't a Mixin, it turns the class into an exception and 
> > > raises it.
> > > 
> > 
> > It doesn't *turn* a class into anything, it simply returns a function, 
> > instead of a new class, and the function raises SkipTest, the point wasn't 
> > the implementation detail, but rather the syntax and pattern at work. 
> > 
> > > django.test.utils.override_settings is a mixin and it's terrible, it 
> > > dynamically creates a new subclass, and overrides 2 methods. It's magic 
> > > and more complex then need be.
> > > 
> > 
> > 
> > a) it's not a mixin
> > b) yes, it creates subclasses, because the alternative is mutating the 
> > original class, which isn't how people generally expect decorators to work, 
> > we expect them to be syntactic sugar for:
> > 
> > A = wrap(*args)(A)
> > 
> > such that we can also do
> > 
> > B = wrap(*args)(A)
> > 
> > and have two classes (or functions).
> > 
> > c) I'm not sure how it's magic, but certainly if it's too complex we'd take 
> > patches to simplify the implementation. 
> > 
> > > 
> > > On Friday, September 16, 2011 at 9:50 PM, Alex Gaynor wrote:
> > > 
> > > 
> > > 
> > > > 
> > > > 
> > > > On Fri, Sep 16, 2011 at 9:47 PM, James Bennett  > > > (mailto:ubernost...@gmail.com)> wrote:
> > > > >  We have the following constraints:
> > > > > 
> > > > >  1. Django supports class-based views.
> > > > > 
> > > > >  2. Django supports function-based views (ultimately these are the 
> > > > > same
> > > > >  thing, which is that Django supports anything as a 'view' so long as
> > > > >  it's callable, accepts an HttpRequest as its first positional 
> > > > > argument
> > > > >  when being called and either returns an HttpResponse or raises an
> > > > >  exception).
> > > > > 
> > > > >  3. The natural way to add processing in/around a class is subclassing
> > > > >  and either overriding or mixing in.
> > > > > 
> > > > >  4. The natural way to add processing in/around around a function is 
> > > > > decorating.
> > > > > 
> > > > >  Any solution to this needs to address those constraints, and allow
> > > > >  code to look and feel natural.
> > > > > 
> > > > >  Based on that, some arrangement which allows the same basic logic to
> > > > >  exist in a "mixin" (I dislike that word) for classes and a decorator
> > > > >  for functions seems most appropriate, even if it does mean having two
> > > > >  ways to do things -- that's a distinction I think we can live with, 
> > > > > as
> > > > >  people will appreciate that it doesn't result in strange-looking 
> > > > > code.
> > > > 
> > > > I agree with all your points, except #3, I think it's an over 
> > > > simplification. There's another way to change a class: class 
> > > > decorators. And there's ample precedent for their use in such a manner, 
> > > > unittest's skip decorators, our own decorators for swapping settings 
> > > > during testing, etc.
> > > > 
> > > > Alex
> > Alex
> 
>  -- 
>  You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
>  To post to this group, send email to django-developers@googlegroups.com 
> (mailto:django-developers@googlegroups.com).
>  To unsubscribe from this group, send email to 
> django-developers+unsubscr...@googlegroups.com 
> (mailto: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-developers@googlegroups.com.
To unsubscribe from this group, send email to 

Re: RFC: "universal" view decorators

2011-09-21 Thread Roald de Vries

Hi all,

Haven't seen a comment on this topic for a few days, so was wondering  
if a core dev can make a design decision yet. I have some spare time  
in the remainder of this week, so if the (universal) decorator- 
approach is chosen, I should be able to finish it shortly (the mixin- 
approach is a bit harder to estimate).


Cheers, Roald


On Sep 17, 2011, at 4:32 AM, Alex Gaynor wrote:

On Fri, Sep 16, 2011 at 10:27 PM, Donald Stufft  wrote:
unittest.skip isn't a Mixin, it turns the class into an exception  
and raises it.



It doesn't *turn* a class into anything, it simply returns a  
function, instead of a new class, and the function raises SkipTest,  
the point wasn't the implementation detail, but rather the syntax  
and pattern at work.


django.test.utils.override_settings is a mixin and it's terrible, it  
dynamically creates a new subclass, and overrides 2 methods. It's  
magic and more complex then need be.


a) it's not a mixin
b) yes, it creates subclasses, because the alternative is mutating  
the original class, which isn't how people generally expect  
decorators to work, we expect them to be syntactic sugar for:


A = wrap(*args)(A)

such that we can also do

B = wrap(*args)(A)

and have two classes (or functions).

c) I'm not sure how it's magic, but certainly if it's too complex  
we'd take patches to simplify the implementation.


On Friday, September 16, 2011 at 9:50 PM, Alex Gaynor wrote:




On Fri, Sep 16, 2011 at 9:47 PM, James Bennett  
 wrote:

We have the following constraints:

1. Django supports class-based views.

2. Django supports function-based views (ultimately these are the  
same

thing, which is that Django supports anything as a 'view' so long as
it's callable, accepts an HttpRequest as its first positional  
argument

when being called and either returns an HttpResponse or raises an
exception).

3. The natural way to add processing in/around a class is  
subclassing

and either overriding or mixing in.

4. The natural way to add processing in/around around a function  
is decorating.


Any solution to this needs to address those constraints, and allow
code to look and feel natural.

Based on that, some arrangement which allows the same basic logic to
exist in a "mixin" (I dislike that word) for classes and a decorator
for functions seems most appropriate, even if it does mean having  
two
ways to do things -- that's a distinction I think we can live  
with, as
people will appreciate that it doesn't result in strange-looking  
code.


I agree with all your points, except #3, I think it's an over  
simplification.  There's another way to change a class: class  
decorators.  And there's ample precedent for their use in such a  
manner, unittest's skip decorators, our own decorators for swapping  
settings during testing, etc.


Alex


Alex


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-17 Thread Reinout van Rees

On 16-09-11 18:08, Javier Guerra Giraldez wrote:

I guess that when you know by heart exactly what does each one do,
they become a pleasure to use.


They *are* a pleasure to use. A colleague switched an app over to class 
based views last week and he got rid of 30% of the code. And it was more 
elegant and readable.


Just mentioning that as I'm real happy with class based views :-)


Reinout

--
Reinout van Reeshttp://reinout.vanrees.org/
rein...@vanrees.org http://www.nelen-schuurmans.nl/
"If you're not sure what to do, make something. -- Paul Graham"

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Donald Stufft
 The main reason I got into the implementation is because I think it's an 
important detail, decorating classes in my opinion is the right thing to do 
when what you are doing is something along the lines of registering the class, 
or doing some sort of validation and raising an exception etc.  

django.test.utils.override_settings when decorating a class is nearly 
functionally equivalent to adding a mixin, it takes the base class, and adds in 
2 methods. It's not actually a mixing, but I meant it's nearly equivalent to a 
mixin (in the TestCase case). I don't think class decorators are the right way 
to do this sort of pseudo mixin in a very opaque way.  

That being said…

Is there a problem with turning the current decorators into mixins, and support 
both decorating the class, and mixing in from the same code base? It's somewhat 
unpythonic in the 2 ways to do it camp, but it'd all be done with 1 code base, 
and would satisfy both camps I believe?  


On Friday, September 16, 2011 at 10:32 PM, Alex Gaynor wrote:

>  
>  
> On Fri, Sep 16, 2011 at 10:27 PM, Donald Stufft  (mailto:donald.stu...@gmail.com)> wrote:
> > unittest.skip isn't a Mixin, it turns the class into an exception and 
> > raises it.
> >  
>  
> It doesn't *turn* a class into anything, it simply returns a function, 
> instead of a new class, and the function raises SkipTest, the point wasn't 
> the implementation detail, but rather the syntax and pattern at work.  
>  
> > django.test.utils.override_settings is a mixin and it's terrible, it 
> > dynamically creates a new subclass, and overrides 2 methods. It's magic and 
> > more complex then need be.
> >  
>  
>  
> a) it's not a mixin
> b) yes, it creates subclasses, because the alternative is mutating the 
> original class, which isn't how people generally expect decorators to work, 
> we expect them to be syntactic sugar for:
>  
> A = wrap(*args)(A)
>  
> such that we can also do
>  
> B = wrap(*args)(A)
>  
> and have two classes (or functions).
>  
> c) I'm not sure how it's magic, but certainly if it's too complex we'd take 
> patches to simplify the implementation.  
>  
> >  
> > On Friday, September 16, 2011 at 9:50 PM, Alex Gaynor wrote:
> >  
> >  
> >  
> > >  
> > >  
> > > On Fri, Sep 16, 2011 at 9:47 PM, James Bennett  > > (mailto:ubernost...@gmail.com)> wrote:
> > > >  We have the following constraints:
> > > >  
> > > >  1. Django supports class-based views.
> > > >  
> > > >  2. Django supports function-based views (ultimately these are the same
> > > >  thing, which is that Django supports anything as a 'view' so long as
> > > >  it's callable, accepts an HttpRequest as its first positional argument
> > > >  when being called and either returns an HttpResponse or raises an
> > > >  exception).
> > > >  
> > > >  3. The natural way to add processing in/around a class is subclassing
> > > >  and either overriding or mixing in.
> > > >  
> > > >  4. The natural way to add processing in/around around a function is 
> > > > decorating.
> > > >  
> > > >  Any solution to this needs to address those constraints, and allow
> > > >  code to look and feel natural.
> > > >  
> > > >  Based on that, some arrangement which allows the same basic logic to
> > > >  exist in a "mixin" (I dislike that word) for classes and a decorator
> > > >  for functions seems most appropriate, even if it does mean having two
> > > >  ways to do things -- that's a distinction I think we can live with, as
> > > >  people will appreciate that it doesn't result in strange-looking code.
> > > >  
> > > >  
> > > >  --
> > > >  "Bureaucrat Conrad, you are technically correct -- the best kind of 
> > > > correct."
> > > >  
> > > >  --
> > > >  You received this message because you are subscribed to the Google 
> > > > Groups "Django developers" group.
> > > >  To post to this group, send email to 
> > > > django-developers@googlegroups.com 
> > > > (mailto:django-developers@googlegroups.com).
> > > >  To unsubscribe from this group, send email to 
> > > > django-developers+unsubscr...@googlegroups.com 
> > > > (mailto:django-developers%2bunsubscr...@googlegroups.com).
> > > >  For more options, visit this group at 
> > > > http://groups.google.com/group/django-developers?hl=en.
> > > >  
> > >  
> > > I agree with all your points, except #3, I think it's an over 
> > > simplification. There's another way to change a class: class decorators. 
> > > And there's ample precedent for their use in such a manner, unittest's 
> > > skip decorators, our own decorators for swapping settings during testing, 
> > > etc.
> > >  
> > > Alex
> > >  
> > > --  
> > > "I disapprove of what you say, but I will defend to the death your right 
> > > to say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
> > > "The people's good is the highest law." -- Cicero
> > >  
> > >  --  
> > >  You received this message because you are subscribed to the Google 
> > > Groups 

Re: RFC: "universal" view decorators

2011-09-16 Thread Justin Holmes
James,

Your case seems to rest on what is "natural" - both in terms of how to
modify existing control structures and as a general goal that our
resulting syntax must "feel natural."

I submit that the way that will "feel natural" is to simply allow
decoration of any view, be it a class or a function, with the
decorator whose name describes precisely the functional modification.
This is simple to read, easy to remember, and, frankly, DRY.


On Fri, Sep 16, 2011 at 7:27 PM, Donald Stufft  wrote:
> unittest.skip isn't a Mixin, it turns the class into an exception and raises
> it.
> django.test.utils.override_settings is a mixin and it's terrible, it
> dynamically creates a new subclass, and overrides 2 methods. It's magic and
> more complex then need be.
>
> On Friday, September 16, 2011 at 9:50 PM, Alex Gaynor wrote:
>
> On Fri, Sep 16, 2011 at 9:47 PM, James Bennett 
> wrote:
>
> We have the following constraints:
>
> 1. Django supports class-based views.
>
> 2. Django supports function-based views (ultimately these are the same
> thing, which is that Django supports anything as a 'view' so long as
> it's callable, accepts an HttpRequest as its first positional argument
> when being called and either returns an HttpResponse or raises an
> exception).
>
> 3. The natural way to add processing in/around a class is subclassing
> and either overriding or mixing in.
>
> 4. The natural way to add processing in/around around a function is
> decorating.
>
> Any solution to this needs to address those constraints, and allow
> code to look and feel natural.
>
> Based on that, some arrangement which allows the same basic logic to
> exist in a "mixin" (I dislike that word) for classes and a decorator
> for functions seems most appropriate, even if it does mean having two
> ways to do things -- that's a distinction I think we can live with, as
> people will appreciate that it doesn't result in strange-looking code.
>
>
> --
> "Bureaucrat Conrad, you are technically correct -- the best kind of
> correct."
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@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.
>
>
> I agree with all your points, except #3, I think it's an over
> simplification.  There's another way to change a class: class decorators.
>  And there's ample precedent for their use in such a manner, unittest's skip
> decorators, our own decorators for swapping settings during testing, etc.
> Alex
>
> --
> "I disapprove of what you say, but I will defend to the death your right to
> say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
> "The people's good is the highest law." -- Cicero
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@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-developers@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.
>



-- 
Justin Holmes

Head Instructor, SlashRoot Collective
SlashRoot: Coffee House and Tech Dojo
60 Main Street
New Paltz, NY 12561
845.633.8330

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Alex Gaynor
On Fri, Sep 16, 2011 at 10:27 PM, Donald Stufft wrote:

> unittest.skip isn't a Mixin, it turns the class into an exception and
> raises it.
>
>
It doesn't *turn* a class into anything, it simply returns a function,
instead of a new class, and the function raises SkipTest, the point wasn't
the implementation detail, but rather the syntax and pattern at work.


> django.test.utils.override_settings is a mixin and it's terrible, it
> dynamically creates a new subclass, and overrides 2 methods. It's magic and
> more complex then need be.
>

a) it's not a mixin
b) yes, it creates subclasses, because the alternative is mutating the
original class, which isn't how people generally expect decorators to work,
we expect them to be syntactic sugar for:

A = wrap(*args)(A)

such that we can also do

B = wrap(*args)(A)

and have two classes (or functions).

c) I'm not sure how it's magic, but certainly if it's too complex we'd take
patches to simplify the implementation.


>  On Friday, September 16, 2011 at 9:50 PM, Alex Gaynor wrote:
>
>
>
> On Fri, Sep 16, 2011 at 9:47 PM, James Bennett wrote:
>
> We have the following constraints:
>
> 1. Django supports class-based views.
>
> 2. Django supports function-based views (ultimately these are the same
> thing, which is that Django supports anything as a 'view' so long as
> it's callable, accepts an HttpRequest as its first positional argument
> when being called and either returns an HttpResponse or raises an
> exception).
>
> 3. The natural way to add processing in/around a class is subclassing
> and either overriding or mixing in.
>
> 4. The natural way to add processing in/around around a function is
> decorating.
>
> Any solution to this needs to address those constraints, and allow
> code to look and feel natural.
>
> Based on that, some arrangement which allows the same basic logic to
> exist in a "mixin" (I dislike that word) for classes and a decorator
> for functions seems most appropriate, even if it does mean having two
> ways to do things -- that's a distinction I think we can live with, as
> people will appreciate that it doesn't result in strange-looking code.
>
>
> --
> "Bureaucrat Conrad, you are technically correct -- the best kind of
> correct."
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@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.
>
>
> I agree with all your points, except #3, I think it's an over
> simplification.  There's another way to change a class: class decorators.
>  And there's ample precedent for their use in such a manner, unittest's skip
> decorators, our own decorators for swapping settings during testing, etc.
>
> Alex
>
> --
> "I disapprove of what you say, but I will defend to the death your right to
> say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
> "The people's good is the highest law." -- Cicero
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@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-developers@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.
>

Alex

-- 
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Donald Stufft
unittest.skip isn't a Mixin, it turns the class into an exception and raises it.

django.test.utils.override_settings is a mixin and it's terrible, it 
dynamically creates a new subclass, and overrides 2 methods. It's magic and 
more complex then need be. 


On Friday, September 16, 2011 at 9:50 PM, Alex Gaynor wrote:

> 
> 
> On Fri, Sep 16, 2011 at 9:47 PM, James Bennett  (mailto:ubernost...@gmail.com)> wrote:
> >  We have the following constraints:
> > 
> >  1. Django supports class-based views.
> > 
> >  2. Django supports function-based views (ultimately these are the same
> >  thing, which is that Django supports anything as a 'view' so long as
> >  it's callable, accepts an HttpRequest as its first positional argument
> >  when being called and either returns an HttpResponse or raises an
> >  exception).
> > 
> >  3. The natural way to add processing in/around a class is subclassing
> >  and either overriding or mixing in.
> > 
> >  4. The natural way to add processing in/around around a function is 
> > decorating.
> > 
> >  Any solution to this needs to address those constraints, and allow
> >  code to look and feel natural.
> > 
> >  Based on that, some arrangement which allows the same basic logic to
> >  exist in a "mixin" (I dislike that word) for classes and a decorator
> >  for functions seems most appropriate, even if it does mean having two
> >  ways to do things -- that's a distinction I think we can live with, as
> >  people will appreciate that it doesn't result in strange-looking code.
> > 
> > 
> >  --
> >  "Bureaucrat Conrad, you are technically correct -- the best kind of 
> > correct."
> > 
> >  --
> >  You received this message because you are subscribed to the Google Groups 
> > "Django developers" group.
> >  To post to this group, send email to django-developers@googlegroups.com 
> > (mailto:django-developers@googlegroups.com).
> >  To unsubscribe from this group, send email to 
> > django-developers+unsubscr...@googlegroups.com 
> > (mailto:django-developers%2bunsubscr...@googlegroups.com).
> >  For more options, visit this group at 
> > http://groups.google.com/group/django-developers?hl=en.
> > 
> 
> I agree with all your points, except #3, I think it's an over simplification. 
> There's another way to change a class: class decorators. And there's ample 
> precedent for their use in such a manner, unittest's skip decorators, our own 
> decorators for swapping settings during testing, etc.
> 
> Alex
> 
> -- 
> "I disapprove of what you say, but I will defend to the death your right to 
> say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
> "The people's good is the highest law." -- Cicero
> 
>  -- 
>  You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
>  To post to this group, send email to django-developers@googlegroups.com 
> (mailto:django-developers@googlegroups.com).
>  To unsubscribe from this group, send email to 
> django-developers+unsubscr...@googlegroups.com 
> (mailto: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-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Alex Gaynor
On Fri, Sep 16, 2011 at 9:47 PM, James Bennett wrote:

> We have the following constraints:
>
> 1. Django supports class-based views.
>
> 2. Django supports function-based views (ultimately these are the same
> thing, which is that Django supports anything as a 'view' so long as
> it's callable, accepts an HttpRequest as its first positional argument
> when being called and either returns an HttpResponse or raises an
> exception).
>
> 3. The natural way to add processing in/around a class is subclassing
> and either overriding or mixing in.
>
> 4. The natural way to add processing in/around around a function is
> decorating.
>
> Any solution to this needs to address those constraints, and allow
> code to look and feel natural.
>
> Based on that, some arrangement which allows the same basic logic to
> exist in a "mixin" (I dislike that word) for classes and a decorator
> for functions seems most appropriate, even if it does mean having two
> ways to do things -- that's a distinction I think we can live with, as
> people will appreciate that it doesn't result in strange-looking code.
>
>
> --
> "Bureaucrat Conrad, you are technically correct -- the best kind of
> correct."
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-developers@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.
>
>
I agree with all your points, except #3, I think it's an over
simplification.  There's another way to change a class: class decorators.
 And there's ample precedent for their use in such a manner, unittest's skip
decorators, our own decorators for swapping settings during testing, etc.

Alex

-- 
"I disapprove of what you say, but I will defend to the death your right to
say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread James Bennett
We have the following constraints:

1. Django supports class-based views.

2. Django supports function-based views (ultimately these are the same
thing, which is that Django supports anything as a 'view' so long as
it's callable, accepts an HttpRequest as its first positional argument
when being called and either returns an HttpResponse or raises an
exception).

3. The natural way to add processing in/around a class is subclassing
and either overriding or mixing in.

4. The natural way to add processing in/around around a function is decorating.

Any solution to this needs to address those constraints, and allow
code to look and feel natural.

Based on that, some arrangement which allows the same basic logic to
exist in a "mixin" (I dislike that word) for classes and a decorator
for functions seems most appropriate, even if it does mean having two
ways to do things -- that's a distinction I think we can live with, as
people will appreciate that it doesn't result in strange-looking code.


-- 
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Donald Stufft
Existing in python != pythonic. (not stating that class decorators aren't 
pythonic, but that argument is flawed)

Just watched the video my thoughts regarding it, and this discussion.

The Augment pattern is a terrible use case for class decorators when you are 
just overriding methods. It's nothing more then Mixins with a different syntax, 
for no reason that I can determine other then someone not liking the way 
declaring a mixin looks, functionally you are still mixing in. What benefit do 
you get from hiding the fact you are really just doing a Mixin? With the code 
Jacob posted there is… 16? (My math might be wrong) different permutations of 
code flow just to support universal decorators.  

The order would matter in the sense that any mixins would have to come before 
the base view class, so:

class ProtectedView(LoginRequired, View)

is valid but

class ProtectedView(View, LoginRequired)

is not.

But this is just python, there's nothing magical about object inheritance and 
in python object inheritance is left to right.

Additionally, as I just thought, there is a way to satisfy both camps. The 
support code I proposed to allow @login_required with Mixins to still work with 
function based views could include the universal decorator approach that Jacob 
included.

That would be a change so that the actual "is this a user, are they logged in 
etc" logic would live in a class that could be mixed in, then the support code 
for functions would use that class and wrap the test portions around the 
function view, and then the universal decorator code could just do the Augment 
pattern.

All camps == Happy? People who prefer mixins can mixin, people who prefer 
@decorators can decorate, everything is supported from one code base.  


On Friday, September 16, 2011 at 1:45 PM, Roald de Vries wrote:

> On Sep 16, 2011, at 6:19 PM, Donald Stufft wrote:
>  
> > Documentation is being worked on, and is orthogonal to the current  
> > discussion of how
> > to handle things like requiring logins with the new CBVs.
>  
> I just watched "Class Decorators: Radically Simple" by Jack Diederich,  
> who wrote the class decorators PEP, and I think it's very useful to  
> watch (25 min.) for this discussion. According to him it is good  
> practice to return the class that is decorated, which I think we  
> should follow, and which solves the biggest practical problems with  
> decorating. Moreover, the fact that class decorators exist indicate  
> that they are pythonic. So +1 for the decorators.
>  
> Considering the mixins: IMHO, the order of base classes shouldn't  
> matter. Can this be satisfied by the mixin-approach?
>  
> @Carl Meyer: I would opt for applying decorators *in* the class, so  
> you can still derive from it. Like::
>  
>  class MyView(View):
>  @classonlymethod
>  def as_view(cls, *args, **kwargs):
>  return login_required(super(MyView, cls).as_view(*args,  
> **kwargs))
>  
> Cheers, Roald
>  
> --  
> You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com 
> (mailto:django-developers@googlegroups.com).
> To unsubscribe from this group, send email to 
> django-developers+unsubscr...@googlegroups.com 
> (mailto: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-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Roald de Vries

On Sep 16, 2011, at 6:19 PM, Donald Stufft wrote:

Documentation is being worked on, and is orthogonal to the current  
discussion of how

to handle things like requiring logins with the new CBVs.


I just watched "Class Decorators: Radically Simple" by Jack Diederich,  
who wrote the class decorators PEP, and I think it's very useful to  
watch (25 min.) for this discussion. According to him it is good  
practice to return the class that is decorated, which I think we  
should follow, and which solves the biggest practical problems with  
decorating. Moreover, the fact that class decorators exist indicate  
that they are pythonic. So +1 for the decorators.


Considering the mixins: IMHO, the order of base classes shouldn't  
matter. Can this be satisfied by the mixin-approach?


@Carl Meyer: I would opt for applying decorators *in* the class, so  
you can still derive from it. Like::


class MyView(View):
@classonlymethod
def as_view(cls, *args, **kwargs):
return login_required(super(MyView, cls).as_view(*args,  
**kwargs))


Cheers, Roald

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Donald Stufft
Documentation is being worked on, and is orthogonal to the current discussion 
of how
to handle things like requiring logins with the new CBVs.



On Friday, September 16, 2011 at 12:08 PM, Javier Guerra Giraldez wrote:

> On Fri, Sep 16, 2011 at 8:34 AM, Reinout van Rees  (mailto:rein...@vanrees.org)> wrote:
> > Watch out, in general, with adding more and more mixins.
> > 
> > I explained just the most basic template CBV to a colleague: there are just
> > two mixins and a base class there, but his eyes already started to glaze
> > over because I had to jump from class to class to class to explain how it
> > all hung together.
> 
> that is my own reaction too when trying to see where in the code is
> everything that i knew from the old generic view functions.
> 
> mixins are a great way to encapsulate specific, well defined behavior;
> but tracing the code is a real chore, even so clean code as Django's.
> I guess that when you know by heart exactly what does each one do,
> they become a pleasure to use.
> 
> So, my two cents: please do the documentation first. once there's a
> single place to find exactly what the purpose, mechanism and span of
> each one, then adding more would be welcomed.
> 
> 
> -- 
> Javier
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Django developers" group.
> To post to this group, send email to django-developers@googlegroups.com 
> (mailto:django-developers@googlegroups.com).
> To unsubscribe from this group, send email to 
> django-developers+unsubscr...@googlegroups.com 
> (mailto: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-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Javier Guerra Giraldez
On Fri, Sep 16, 2011 at 8:34 AM, Reinout van Rees  wrote:
> Watch out, in general, with adding more and more mixins.
>
> I explained just the most basic template CBV to a colleague: there are just
> two mixins and a base class there, but his eyes already started to glaze
> over because I had to jump from class to class to class to explain how it
> all hung together.

that is my own reaction too when trying to see where in the code is
everything that i knew from the old generic view functions.

mixins are a great way to encapsulate specific, well defined behavior;
but tracing the code is a real chore, even so clean code as Django's.
I guess that when you know by heart exactly what does each one do,
they become a pleasure to use.

So, my two cents: please do the documentation first.  once there's a
single place to find exactly what the purpose, mechanism and span of
each one, then adding more would be welcomed.


-- 
Javier

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Donald von Stufft
In response to Reinout:

For the majority of people they won't have to care about what the
LoginRequired Mixin is doing,
they'll just add it to their class definition and that will be the end of
it, the same as if they were decorating it. The major
differences being now all the "things" that declare how this CBV behaves
exist in one spot.If they don't need to customize
the behavior of it, they don't have to, it will just work. If they do need
to customize then the option is there to do so without
having to copy/paste the code from Django into a utils.py and doing it
themselves.

Basically a decorator adds the same amount of complexity into the stack, you
just can't get at it because it's essentially a black box,
so you can't tweak it.

In response to Carl Meyer:

Wrapping the view function like that is uglier (to me atleast) then just
adding a mixin. There's no reason to do
it like that and if that's the only option then I would prefer a decorator.
The goal is to make it simple, easy
and readable. To do so, In my opinion, we need to keep everything about a
class, with the class, wrapping
it in the urls.py, or down further in the views.py just gives yet another
place to look to find out why
a view is doing X.

In response to Mikhail:

Funcitions ARE less customizable then a nicely written class, tell me, with
the current login_required
how would you make it return a 403 instead of a redirect to login? Or how
would you make it check
that the user is_active as well as is_authorized? (The answer as far as I am
aware is you would wrap
them again, adding another layer of indirection). There is 0 reason why you
can't support both
@login_required and LoginRequired from the exact same code base, with just a
little bit of a wrapper
(most of which already exists and is required for the non class case
anyways).


In response to Łukasz:

I disagree that preconditions are not modifying the functionality of a CBV.
it's changing what
can or cannot be returned from my view, it's changing how my view behaves,
and from
what I can tell, all of the current patches/implementations do it in a way
that is basically
what sub classing would do, just more opaquely and more magically.

In regards to the declaring the decorators in a list:

I'm generally -1 on that. It doesn't solve
any of the problems that decorating has (well other then how to make them
work with classes),
and it is different from "normal" python conventions.

On Fri, Sep 16, 2011 at 9:34 AM, Reinout van Rees wrote:

> On 15-09-11 23:27, Donald Stufft wrote:
>
>> tl;dr; Using Mixins to add in functionality to a CBV makes way more
>> sense then using a decorator which is essentially going to be doing the
>> same thing as a mixing would, it just makes finding what is going on
>> harder, makes customizing the decorator harder and/or uglier, and goes
>> against the way functionality is currently added to a CBV.
>>
>
> Watch out, in general, with adding more and more mixins.
>
> I explained just the most basic template CBV to a colleague: there are just
> two mixins and a base class there, but his eyes already started to glaze
> over because I had to jump from class to class to class to explain how it
> all hung together.
>
> Throw in a LoginRequiredMixin and SomeOtherMixin and you have the risk that
> it turns into a big pile of un-debuggable code. Too many places to look.
> Your mind's main working memory has a stack size between 6 and 8: you just
> cannot juggle 3 base classes, 4 mixins and 2 method names at the same time.
>
> => A mixin isn't necessarily clearer than a decorator.
>
>
> Reinout
>
> --
> Reinout van Reeshttp://reinout.vanrees.org/
> rein...@vanrees.org 
> http://www.nelen-schuurmans.**nl/
> "If you're not sure what to do, make something. -- Paul Graham"
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to 
> django-developers@**googlegroups.com
> .
> To unsubscribe from this group, send email to
> django-developers+unsubscribe@**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-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Reinout van Rees

On 15-09-11 23:27, Donald Stufft wrote:

tl;dr; Using Mixins to add in functionality to a CBV makes way more
sense then using a decorator which is essentially going to be doing the
same thing as a mixing would, it just makes finding what is going on
harder, makes customizing the decorator harder and/or uglier, and goes
against the way functionality is currently added to a CBV.


Watch out, in general, with adding more and more mixins.

I explained just the most basic template CBV to a colleague: there are 
just two mixins and a base class there, but his eyes already started to 
glaze over because I had to jump from class to class to class to explain 
how it all hung together.


Throw in a LoginRequiredMixin and SomeOtherMixin and you have the risk 
that it turns into a big pile of un-debuggable code. Too many places to 
look. Your mind's main working memory has a stack size between 6 and 8: 
you just cannot juggle 3 base classes, 4 mixins and 2 method names at 
the same time.


=> A mixin isn't necessarily clearer than a decorator.


Reinout

--
Reinout van Reeshttp://reinout.vanrees.org/
rein...@vanrees.org http://www.nelen-schuurmans.nl/
"If you're not sure what to do, make something. -- Paul Graham"

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Daniel Moisset
On Thu, Sep 15, 2011 at 6:27 PM, Donald Stufft wrote:

> Gonna add this in here as well as ticket #14512
>
> I think using decorators to modify the way a CBV behaves is the wrong way
> to go about it, my reasoning is:
>
> 1) Decorators on functions make sense, since the only way to modify a
> function is to wrap it, so decorators provide a shortcut to do so.
>
> 2) There's already a standard way to add functionality to a class that has
> existed for a long time, is well tested, and as a bonus is something that
> people new to python but not new to programming will understand immediately.
>

I like the arguments here. On the other hand, I like Jacob's

«I'd like to convert all the view decorators built into Django to be
"universal" -- so they'll work to decorate *any* view, whether a function,
method, or class»

So what about providing something roughly like:

###
class LoginRequired():
# magic to check for user credentials, in a CBV style
...
def __call__(self, wrap_me):
# standard magic to apply the CBV decoration to a standard function
# This might also include Jacob's trick to work with methods too
...

login_required = LoginRequired # backwards compatibility
###

Actually that implementation of __call__ is probably the same for every
decorator, so Django can provide something more like:

###
from django.something import BaseViewDecorator
class LoginRequired(BaseViewDecorator):
# magic to check for user credentials, in a CBV style
...

login_required = LoginRequired # backwards compatibility
###

With that, user code can look like:

class SomeProtectedView(LoginRequired, TemplateView):
... as usual...

@login_required()
def some_other_protected_view(request, arg1, arg2):
as usual...

class NotAView():
@login_required()
def view_method(self, request, arg1, arg2):
   ... as usual ...

Which uses the "obvious" way of extension (decorators for functions/methods
and Mixins for classes) in each case (i.e., what Donald wants), but allows
implementing the decorator once, and not having to have specific wrappers
for each kind of view (universality, like Jacob wants).

This is a rough proposal to put another idea on the table, I know there's
the gotcha of the "login_required = LoginRequired" (which is to keep
backwards compatibility. Other alternative is rename the class to a non-PEP8
lowercase name). And I know there's the gotcha of having to use
login_required() instead of just login_required[1].

Does this sound like a good compromise between both positions?

Regards,
   Daniel

[1] I always found a bit irritating and antipythonic to have @login_required
as an alias for login_required() ; or in general, the behavior for
decorators with optional args, which besides needs to be coded explicitly
(DRY violation!) to behave like standard decorators. I know that changing
that would break a lot of code, and it's a story for another ticket, but I
couldn't miss the chance to complain :)

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Roald de Vries

On Sep 16, 2011, at 11:42 AM, Łukasz Rekucki wrote:


On 16 September 2011 10:17, Roald de Vries  wrote:

On Sep 16, 2011, at 12:11 AM, Łukasz Rekucki wrote:

I would like to also comment on the new approach in that ticket.
Making a shallow copy of a class is *MAGIC* to me. It breaks the  
basic
invariant "issubsclass(decorator(A), A) == True". This is  
important if

you're planing to use this as "B = decorator()(A)" (and you are,
'cause the whole point of not modifying the original class is to  
allow

safely doing this), as you quickly end up with weird alternate
hierarchies. So, please don't do that :)


I agree that in an ideal world we'd have a better solution. On the  
other
hand, the cbv-decorator using inheritance just didn't work, and the  
one
using shallow copies does (if you don't use B=decorator()(A),  
indeed).


The one that just alters the given class also works, has no magic and
has the same limitation - dont' do "B = decorator(A)".


To show that this is actually a Python-thing, an analogous non- 
decorator-example:


>>> class A(Base):
... def method(self, *args, **kwargs):
... super(A, self).method(*args, **kwargs)
...
... B = A
... A = SomeOtherClass
...
... B.method()
Traceback (most recent call last):
...
TypeError: unbound method method() must be called with A instance  
as first argument (got nothing instead)


So IMHO it's "pythonic" to leave responsibility to use the decorator  
in the right way to the programmer.


Cheers, Roald

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Mikhail Korobov
I don't agree with most points because they are assuming functions are less 
fancy, less customizable, less clean, more complex, etc than classes and 
this is not true (but let's not start FP vs OOP holywar here, FP and OOP are 
friends in python).

I like Jacob's proposal because there should be one way to do something and 
this way can't be mixins because they can't work with functions. Decorators 
can work with classes, this is standard python and it will be nice to have 
them work with class-based views.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To view this discussion on the web visit 
https://groups.google.com/d/msg/django-developers/-/ZCeB1UehSSQJ.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Łukasz Rekucki
On 16 September 2011 10:17, Roald de Vries  wrote:
> On Sep 16, 2011, at 12:11 AM, Łukasz Rekucki wrote:
>>
>> As the ticket creator I feel obligated to reply :)
>
> Me (as the poster of the latest patch) too :)

Nice to meet you.

>
>> Thinking about it now, it does look kinda ugly. It's mainly because I
>> was focus on how to reuse existing decorators in CBV context. It
>> shouldn't be to hard  to make the "view_decorator" a meta-decorator
>> (or a factory function as you call it) that turns login_required to
>> something that is both a class and function decorator. Methods are
>> still out-of-bounds for non-runtime introspection, but after almost 1
>> year of using CBV, I never needed to decorate a method.
>>
>> I would like to also comment on the new approach in that ticket.
>> Making a shallow copy of a class is *MAGIC* to me. It breaks the basic
>> invariant "issubsclass(decorator(A), A) == True". This is important if
>> you're planing to use this as "B = decorator()(A)" (and you are,
>> 'cause the whole point of not modifying the original class is to allow
>> safely doing this), as you quickly end up with weird alternate
>> hierarchies. So, please don't do that :)
>
> I agree that in an ideal world we'd have a better solution. On the other
> hand, the cbv-decorator using inheritance just didn't work, and the one
> using shallow copies does (if you don't use B=decorator()(A), indeed).

The one that just alters the given class also works, has no magic and
has the same limitation - dont' do "B = decorator(A)". Honestly, I
never needed that, but I understand some people might. Also note, that
the "super() problem" is fixed in Python 3, where super() determines
the base class from the call frame instead of referencing a global
variable.

>
> If this doesn't exist, then the view_decorator still has a right to live.
>
> So what would have to be created is a meta-decorator universal_decorator,
> replacing or using view_decorator, such that
> universal_decorator(login_required) is the actual universal decorator. This
> would be applied to all django-decorators, but I think it would be good to
> also allow applying universal_decorator to an already universal decorator,
> such that:
>
>    # after this...
>    login_required = universal_decorator(login_required)
>    # ... this will hold
>    assert login_required == universal_decorator(login_required)
>
>

+1

-- 
Ł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-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Łukasz Rekucki
> I think the syntax should be something like:
>
> from django.contrib.auth.decorators import login_required,
> permission_required
>
> class ProtectedView(MyView):
>    view_decorators = [login_required, permission_required('foo.can_do_bar')]
>

There was a similar proposal on this list before introducing CBV. It
didn't gain much love.

-- 
Ł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-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Anssi Kääriäinen

On 09/16/2011 11:20 AM, Roald de Vries wrote:

On Sep 16, 2011, at 10:08 AM, Jonathan Slenders wrote:


class ProtectedView(MyView):
login_required = True

Where 'MyView' checks all the view's options during a dispatch. Never
liked inheritance with multiple base classes...

How would I create my own 'decorators' in this approach?

Cheers, Roald


I think the syntax should be something like:

from django.contrib.auth.decorators import login_required, 
permission_required


class ProtectedView(MyView):
view_decorators = [login_required, 
permission_required('foo.can_do_bar')]


The difference to actually decorating the class is small, but I think 
the above is a bit more explicit about what is happening.


Mixins sound like a good alternative, but does this just move the 
problem in to mixin class instead of the base view class? That is, how 
should the mixin class be implemented taking in account that you should 
be able to decorate the function with multiple decorators? It seems you 
would need something like ViewDecoratorMixinBase to take care of all the 
dirty details...


 - Anssi

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Roald de Vries

On Sep 16, 2011, at 10:08 AM, Jonathan Slenders wrote:


class ProtectedView(MyView):
   login_required = True

Where 'MyView' checks all the view's options during a dispatch. Never
liked inheritance with multiple base classes...


How would I create my own 'decorators' in this approach?

Cheers, Roald

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Roald de Vries

Hi,

On Sep 16, 2011, at 12:11 AM, Łukasz Rekucki wrote:


Hi,

On 15 September 2011 22:44, Jacob Kaplan-Moss   
wrote:


#14512 proposes a adding another view-decorator-factory for  
decorating

class-based views, which would turn the above into::

   @class_view_decorator(login_required)
   class MyView(View):
   ...

This makes me less sad, but still sad. Factory functions. Ugh.


As the ticket creator I feel obligated to reply :)


Me (as the poster of the latest patch) too :)


Thinking about it now, it does look kinda ugly. It's mainly because I
was focus on how to reuse existing decorators in CBV context. It
shouldn't be to hard  to make the "view_decorator" a meta-decorator
(or a factory function as you call it) that turns login_required to
something that is both a class and function decorator. Methods are
still out-of-bounds for non-runtime introspection, but after almost 1
year of using CBV, I never needed to decorate a method.

I would like to also comment on the new approach in that ticket.
Making a shallow copy of a class is *MAGIC* to me. It breaks the basic
invariant "issubsclass(decorator(A), A) == True". This is important if
you're planing to use this as "B = decorator()(A)" (and you are,
'cause the whole point of not modifying the original class is to allow
safely doing this), as you quickly end up with weird alternate
hierarchies. So, please don't do that :)


I agree that in an ideal world we'd have a better solution. On the  
other hand, the cbv-decorator using inheritance just didn't work, and  
the one using shallow copies does (if you don't use B=decorator()(A),  
indeed).


As Donald mentioned, the more elegant solution for classes is to use  
base classes (mixins) instead of class decorators. I'm not sure though  
if every decorator can be replaced by a mixin, and if the order of  
these mixins can be arbitrary in that case (which I believe is  
desirable). Can anybody comment on that?



I believe, however, I've figured out a different technique to make
this work: don't try to detect bound versus unbound methods, but
instead look for the HttpRequest object. It'll either be args[0] if
the view's a function, or args[1] if the view's a method. This
technique won't work for any old decorator, but it *will* work (I
think) for any decorator *designed to be applied only to views*.


+1 on this. I would be a bit concerned about this vs. future
implementation of generator-based-views that allow some kind of
response streaming (is someone working on that?).


I've written a proof-of-concept patch to @login_required (well,
@user_passes_test, actually):

   https://gist.github.com/1220375


Did I already mention creating a subclass in the class decorator
breaks super ;) [https://gist.github.com/643536]

Can I get some thoughts on this technique and some feedback on  
whether

it's OK to apply to every decorator built into Django?


It would be great to have that meta-decorator, so everyone else could
upgrade their decorators just by decorating them.


If this doesn't exist, then the view_decorator still has a right to  
live.


So what would have to be created is a meta-decorator  
universal_decorator, replacing or using view_decorator, such that  
universal_decorator(login_required) is the actual universal decorator.  
This would be applied to all django-decorators, but I think it would  
be good to also allow applying universal_decorator to an already  
universal decorator, such that:


# after this...
login_required = universal_decorator(login_required)
# ... this will hold
assert login_required == universal_decorator(login_required)







--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Jonathan Slenders
I agree with Donald's reasoning. Decorators belong to functional
programming, not to OO programming. Though, I still like to keep using
functions for my own views and there are decorators appropriate.

But if you go class based, you better use inheritance. However,
instead of mix-ins, I'd rather prefer a more declarative approach,
like this:

class ProtectedView(MyView):
login_required = True

Where 'MyView' checks all the view's options during a dispatch. Never
liked inheritance with multiple base classes...

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-16 Thread Jonas H.

On 09/15/2011 11:27 PM, Donald Stufft wrote:

tl;dr; Using Mixins to add in functionality to a CBV makes way more sense then
using a decorator which is essentially going to be doing the same thing as a
mixing would, it just makes finding what is going on harder, makes customizing
the decorator harder and/or uglier, and goes against the way functionality is
currently added to a CBV.


That sounds a lot better to me.

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-developers@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: RFC: "universal" view decorators

2011-09-15 Thread Łukasz Rekucki
On 15 September 2011 23:27, Donald Stufft  wrote:
> Gonna add this in here as well as ticket #14512
> I think using decorators to modify the way a CBV behaves is the wrong way to
> go about it, my reasoning is:
> 1) Decorators on functions make sense, since the only way to modify a
> function is to wrap it, so decorators provide a shortcut to do so.
> 2) There's already a standard way to add functionality to a class that has
> existed for a long time, is well tested, and as a bonus is something that
> people new to python but not new to programming will understand immediately.
> 3) Decorating a class is different to how adding any other kind of
> functionality to a CBV is done.
> 4) Customizability. Currently your only options to change the way one of the
> current decorators work is to rewrite it, unless the original author of the
> decorator thought of your use case, thought it was valid, and added a
> flag/option for it.
>     - This can be alleviated by using Object based decorators instead of
> function based, but that becomes uglier too because then you end up having
> to first subclass the decorator, then decorate the class.
> 5) Conceptually all the decorator really is doing is mixing in one method
> anyways, it's just hiding this away and adding more code and complexity, and
> reducing customizability and readability for the sole purpose of being able
> to use an @ symbol.
> 6) Forwards compatibility. The function based generic views have been
> deprecated. While function based views will probably always be supported,
> personally I think CBV's are the way forward, and it makes sense to have
> them be first class citizens as regards to adding in things like requiring
> logins, with the function based views having the helper code.
> 7) Backwards compatibility. With a (much simpler) bit of wrapper code, the
> decorators can easily still be supported (login_required etc) and can
> internally use the classes and present the same interface as they used to.
> The major difference being, now they'll be able to subclass them and
> customize tiny bits of it as well.
>
> To just expand a little more in general on this
> @login_required
> class ProtectedView(TemplateView):
>     template_name = "secret.html"
> vs
> class ProtectedView(LoginRequired, TemplateView):
>     template_name = "secret.html"
> In the first, the "things" defining how this CBV behaves are in two
> locations, which makes it harder to read, additionally you end up with a
> function that wraps a function that wraps a function (to some degree of
> wrapping) to actually get all of this to work. Each layer of wrapping adds
> another layer of indirection, and adds another layer of confusion for
> someone attempting to read the code.
> I really dumbed down example would be https://gist.github.com/1220512 .
> Obviously that isn't working code, but it gives a general idea of what I
> mean. Obviously if that is the path that is chosen it will need to be
> cleaned up and made to actually work.
> Hopefully this email makes sense. I just hope that since CBV's are new we
> can use this as an opportunity to do things in a cleaner, consistent and
> more generic way instead of continuing the old way (that made sense for
> functions) for the sake of doing it the same way.
> tl;dr; Using Mixins to add in functionality to a CBV makes way more sense
> then using a decorator which is essentially going to be doing the same thing
> as a mixing would, it just makes finding what is going on harder, makes
> customizing the decorator harder and/or uglier, and goes against the way
> functionality is currently added to a CBV.
>
> On Thursday, September 15, 2011 at 4:44 PM, Jacob Kaplan-Moss wrote:
>
> Hi folks --
>
> I'd like to convert all the view decorators built into Django to be
> "universal" -- so they'll work to decorate *any* view, whether a
> function, method, or class. I believe I've figured out a technique for
> this, but I'd like some feedback on the approach before I dive in too
> deep.
>
> Right now view decorators (e.g. @login_required) only work on
> functions. If you want to use a decorator on a method then you need to
> "convert" the decorator using method_decorator(original_decorator).
> You can't use view decorators on class-based views at all. This means
> making a class-based view require login requires this awesomeness::
>
> class MyView(View):
> @method_decorator(login_required)
> def dispatch(self, *args, **kwargs):
> return super(MyView, self.dispatch(*args, **kwargs)
>
> This makes me sad. It's really counter-intuitive and relies on a
> recognizing that functions and methods are different to even know to
> look for method_decorator.
>
> #14512 proposes a adding another view-decorator-factory for decorating
> class-based views, which would turn the above into::
>
> @class_view_decorator(login_required)
> class MyView(View):
> ...
>
> This makes me less sad, but still sad. Factory functions. Ugh.
>
> I want 

Re: RFC: "universal" view decorators

2011-09-15 Thread Łukasz Rekucki
2011/9/16 Łukasz Rekucki :
> Hi,
>> I believe, however, I've figured out a different technique to make
>> this work: don't try to detect bound versus unbound methods, but
>> instead look for the HttpRequest object. It'll either be args[0] if
>> the view's a function, or args[1] if the view's a method. This
>> technique won't work for any old decorator, but it *will* work (I
>> think) for any decorator *designed to be applied only to views*.
>
> +1 on this. I would be a bit concerned about this vs. future
> implementation of generator-based-views that allow some kind of
> response streaming (is someone working on that?).

Omit this part, It's late and I read HttpResponse by mistake ;), which
of course doesn't make any sense.

-- 
Ł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-developers@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: RFC: "universal" view decorators

2011-09-15 Thread Łukasz Rekucki
Hi,

On 15 September 2011 22:44, Jacob Kaplan-Moss  wrote:
>
> #14512 proposes a adding another view-decorator-factory for decorating
> class-based views, which would turn the above into::
>
>    @class_view_decorator(login_required)
>    class MyView(View):
>        ...
>
> This makes me less sad, but still sad. Factory functions. Ugh.

As the ticket creator I feel obligated to reply :)

Thinking about it now, it does look kinda ugly. It's mainly because I
was focus on how to reuse existing decorators in CBV context. It
shouldn't be to hard  to make the "view_decorator" a meta-decorator
(or a factory function as you call it) that turns login_required to
something that is both a class and function decorator. Methods are
still out-of-bounds for non-runtime introspection, but after almost 1
year of using CBV, I never needed to decorate a method.

I would like to also comment on the new approach in that ticket.
Making a shallow copy of a class is *MAGIC* to me. It breaks the basic
invariant "issubsclass(decorator(A), A) == True". This is important if
you're planing to use this as "B = decorator()(A)" (and you are,
'cause the whole point of not modifying the original class is to allow
safely doing this), as you quickly end up with weird alternate
hierarchies. So, please don't do that :)

>
> I believe, however, I've figured out a different technique to make
> this work: don't try to detect bound versus unbound methods, but
> instead look for the HttpRequest object. It'll either be args[0] if
> the view's a function, or args[1] if the view's a method. This
> technique won't work for any old decorator, but it *will* work (I
> think) for any decorator *designed to be applied only to views*.

+1 on this. I would be a bit concerned about this vs. future
implementation of generator-based-views that allow some kind of
response streaming (is someone working on that?).

>
> I've written a proof-of-concept patch to @login_required (well,
> @user_passes_test, actually):
>
>    https://gist.github.com/1220375
>

Did I already mention creating a subclass in the class decorator
breaks super ;) [https://gist.github.com/643536]


> Can I get some thoughts on this technique and some feedback on whether
> it's OK to apply to every decorator built into Django?
>

It would be great to have that meta-decorator, so everyone else could
upgrade their decorators just by decorating them.

-- 
Ł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-developers@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: RFC: "universal" view decorators

2011-09-15 Thread Donald Stufft
Gonna add this in here as well as ticket #14512

I think using decorators to modify the way a CBV behaves is the wrong way to go 
about it, my reasoning is:

1) Decorators on functions make sense, since the only way to modify a function 
is to wrap it, so decorators provide a shortcut to do so.

2) There's already a standard way to add functionality to a class that has 
existed for a long time, is well tested, and as a bonus is something that 
people new to python but not new to programming will understand immediately.

3) Decorating a class is different to how adding any other kind of 
functionality to a CBV is done.

4) Customizability. Currently your only options to change the way one of the 
current decorators work is to rewrite it, unless the original author of the 
decorator thought of your use case, thought it was valid, and added a 
flag/option for it.
 - This can be alleviated by using Object based decorators instead of function 
based, but that becomes uglier too because then you end up having to first 
subclass the decorator, then decorate the class.

5) Conceptually all the decorator really is doing is mixing in one method 
anyways, it's just hiding this away and adding more code and complexity, and 
reducing customizability and readability for the sole purpose of being able to 
use an @ symbol.

6) Forwards compatibility. The function based generic views have been 
deprecated. While function based views will probably always be supported, 
personally I think CBV's are the way forward, and it makes sense to have them 
be first class citizens as regards to adding in things like requiring logins, 
with the function based views having the helper code.

7) Backwards compatibility. With a (much simpler) bit of wrapper code, the 
decorators can easily still be supported (login_required etc) and can 
internally use the classes and present the same interface as they used to. The 
major difference being, now they'll be able to subclass them and customize tiny 
bits of it as well.


To just expand a little more in general on this

@login_required
class ProtectedView(TemplateView):
 template_name = "secret.html"

vs

class ProtectedView(LoginRequired, TemplateView):
 template_name = "secret.html"

In the first, the "things" defining how this CBV behaves are in two locations, 
which makes it harder to read, additionally you end up with a function that 
wraps a function that wraps a function (to some degree of wrapping) to actually 
get all of this to work. Each layer of wrapping adds another layer of 
indirection, and adds another layer of confusion for someone attempting to read 
the code.

I really dumbed down example would be https://gist.github.com/1220512 . 
Obviously that isn't working code, but it gives a general idea of what I mean. 
Obviously if that is the path that is chosen it will need to be cleaned up and 
made to actually work.

Hopefully this email makes sense. I just hope that since CBV's are new we can 
use this as an opportunity to do things in a cleaner, consistent and more 
generic way instead of continuing the old way (that made sense for functions) 
for the sake of doing it the same way.

tl;dr; Using Mixins to add in functionality to a CBV makes way more sense then 
using a decorator which is essentially going to be doing the same thing as a 
mixing would, it just makes finding what is going on harder, makes customizing 
the decorator harder and/or uglier, and goes against the way functionality is 
currently added to a CBV. 


On Thursday, September 15, 2011 at 4:44 PM, Jacob Kaplan-Moss wrote:

> Hi folks --
> 
> I'd like to convert all the view decorators built into Django to be
> "universal" -- so they'll work to decorate *any* view, whether a
> function, method, or class. I believe I've figured out a technique for
> this, but I'd like some feedback on the approach before I dive in too
> deep.
> 
> Right now view decorators (e.g. @login_required) only work on
> functions. If you want to use a decorator on a method then you need to
> "convert" the decorator using method_decorator(original_decorator).
> You can't use view decorators on class-based views at all. This means
> making a class-based view require login requires this awesomeness::
> 
>  class MyView(View):
>  @method_decorator(login_required)
>  def dispatch(self, *args, **kwargs):
>  return super(MyView, self.dispatch(*args, **kwargs)
> 
> This makes me sad. It's really counter-intuitive and relies on a
> recognizing that functions and methods are different to even know to
> look for method_decorator.
> 
> #14512 proposes a adding another view-decorator-factory for decorating
> class-based views, which would turn the above into::
> 
>  @class_view_decorator(login_required)
>  class MyView(View):
>  ...
> 
> This makes me less sad, but still sad. Factory functions. Ugh.
> 
> I want @login_required to work for anything::
> 
>  @login_required
>  class MyView(View):
>  ...
> 
>  class Something(object):
>  @login_required
> 

RFC: "universal" view decorators

2011-09-15 Thread Jacob Kaplan-Moss
Hi folks --

I'd like to convert all the view decorators built into Django to be
"universal" -- so they'll work to decorate *any* view, whether a
function, method, or class. I believe I've figured out a technique for
this, but I'd like some feedback on the approach before I dive in too
deep.

Right now view decorators (e.g. @login_required) only work on
functions. If you want to use a decorator on a method then you need to
"convert" the decorator using method_decorator(original_decorator).
You can't use view decorators on class-based views at all. This means
making a class-based view require login requires this awesomeness::

class MyView(View):
@method_decorator(login_required)
def dispatch(self, *args, **kwargs):
return super(MyView, self.dispatch(*args, **kwargs)

This makes me sad. It's really counter-intuitive and relies on a
recognizing that functions and methods are different to even know to
look for method_decorator.

#14512 proposes a adding another view-decorator-factory for decorating
class-based views, which would turn the above into::

@class_view_decorator(login_required)
class MyView(View):
...

This makes me less sad, but still sad. Factory functions. Ugh.

I want @login_required to work for anything::

@login_required
class MyView(View):
...

class Something(object):
@login_required
def my_view(self, request):
...

@login_required
def my_view(request):
...


Now, back in the day we somewhat had this: decorators tried to work
with both functions and methods. The technique turned out not to work
(see #12804) and was removed in [12399]. See
http://groups.google.com/group/django-developers/browse_thread/thread/3024b14a47f5404d
for the discussion.

I believe, however, I've figured out a different technique to make
this work: don't try to detect bound versus unbound methods, but
instead look for the HttpRequest object. It'll either be args[0] if
the view's a function, or args[1] if the view's a method. This
technique won't work for any old decorator, but it *will* work (I
think) for any decorator *designed to be applied only to views*.

I've written a proof-of-concept patch to @login_required (well,
@user_passes_test, actually):

https://gist.github.com/1220375

The test suite passes with this, with one exception:
https://code.djangoproject.com/browser/django/trunk/tests/regressiontests/decorators/tests.py#L87.
I maintain that this test is broken and should be using RequestFactory
instead.

Can I get some thoughts on this technique and some feedback on whether
it's OK to apply to every decorator built into Django?

Jacob

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@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.