Class based generic views in 1.3?
Jari Pennanen
without success. (Though syndication views seems to be class based now
at least.)
If I figured out correctly the class based generic views does not land
on 1.2, so I was wondering are they planned to 1.3? Those are rather
important after all...
--
You received this message because you are subscribed to the Google Groups "Django developers" group.
To post to this group, send email to django-d...@googlegroups.com.
To unsubscribe from this group, send email to django-develop...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
Russell Keith-Magee
> I've been trying to figure out the state of class based generic views
> without success. (Though syndication views seems to be class based now
> at least.)
>
> If I figured out correctly the class based generic views does not land
> on 1.2, so I was wondering are they planned to 1.3? Those are rather
> important after all...
features that everyone agrees is a good idea, but someone needs to
actually write the code. Jacob made the most recent attempt at an
implementation that I'm aware of [1], but as the changelog indicates,
he hasn't made any recent updates.
Syndication-based views became class-based because of the good work of
Ben Firshman. He worked up a very good patch, submitted it in time for
the feature freeze, bugged a couple of core developers for reviews,
and it was committed.
Are class-based views planned for 1.3? Well, we haven't done any
formal planning for 1.3 yet, but I'm going to guess that the 1.3 high
priority feature list will essentially be "the features that got
dropped from 1.2", so in all likelihood, yes. However, that doesn't
mean that they will definitely be in 1.3 - someone still needs to do
the implementation. If you really want class based generic views, be
like Ben and make it happen -- show us the code!.
[1] http://github.com/jacobian/django/tree/class-based-generic-views/django/views/generic2
Yours,
Russ Magee %-)
Jari Pennanen
On 11 touko, 03:37, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
>
> [1] http://github.com/jacobian/django/tree/class-based-generic-views/django/views/generic2
one that will be merged. Though if it needs to be improved, I think
Jacobian could *list the problems in his implementation* so others can
try to improve on them, as he probably knows that code best.
From that point of view, I don't see a reason in rewriting the class
based generic-views as there is already Jacobian's take on it.
Russell Keith-Magee
>
> On 11 touko, 03:37, Russell Keith-Magee <russ...@keith-magee.com>
> wrote:
>> show us the code!.
>>
>> [1] http://github.com/jacobian/django/tree/class-based-generic-views/django/views/generic2
>
> Yes, well. I have been in slight assumption that the above code is the
> one that will be merged. Though if it needs to be improved, I think
> Jacobian could *list the problems in his implementation* so others can
> try to improve on them, as he probably knows that code best.
>
> From that point of view, I don't see a reason in rewriting the class
> based generic-views as there is already Jacobian's take on it.
implementation isn't complete. The edit views are still just stubs.
I've also seen comments about consistency between Jacob's
implementation and the class-based syndication views that are in
trunk.
That means there is still work to do, and someone needs to do it. If
you're going to volunteer to do the work, I don't think Jacob would
complain about getting assistance.
Yours,
Russ Magee %-)
Ben Firshman
On 11 May 2010, at 01:37, Russell Keith-Magee wrote:
>
>
> Are class-based views planned for 1.3? Well, we haven't done any
> formal planning for 1.3 yet, but I'm going to guess that the 1.3 high
> priority feature list will essentially be "the features that got
> dropped from 1.2", so in all likelihood, yes. However, that doesn't
> mean that they will definitely be in 1.3 - someone still needs to do
> the implementation. If you really want class based generic views, be
> like Ben and make it happen -- show us the code!.
This is something I've been thinking about a lot, and I'll probably dive into it at the Djangocon.eu sprints. Feel free to make a start Jari. ;)
Ben
fitzgen
base GenericView class since last summer.
http://github.com/fitzgen/class_views/blob/master/views.py
Things that people haven't really mentioned, but need addressing, that
we had to find solutions for:
* How to do redirects in class based views? See lines 71-86 and 166.
* How to decorate the __call__ method without doing a pointless
override of __call__ that just calls super so that you have something
to @decorate on top of. Check out the meta class on line 175. This
allows you to specify 'decorators = [login_required,
permission_required("polls.can_vote")]' on your subclass. I showed
this to Jacob at DjangoSki, and he seemed positive.
* How to decorate methods, when the decorator expects the first
argument to be request, and not self. See line 8. Ideally though,
Django's decorators would handle this, rather than forcing the use of
decorate_method_with on to the end users.
* How to deal with state and self. I have written an instantiator that
wraps the view class and instantiates a new instance of the class for
every request so that everything is thread safe. This works, but
agian, it would be nice if Django checked to see if the callable being
linked to a url route is a class, in which case it would handle the
instantiation of a new instance for every request. See line 188.
Excited to get this stuff in to 1.3!
_Nick_
David Larlet
Le 12 mai 2010 à 22:16, Ben Firshman a écrit :
>
> On 11 May 2010, at 01:37, Russell Keith-Magee wrote:
>>
>>
>> Are class-based views planned for 1.3? Well, we haven't done any
>> formal planning for 1.3 yet, but I'm going to guess that the 1.3 high
>> priority feature list will essentially be "the features that got
>> dropped from 1.2", so in all likelihood, yes. However, that doesn't
>> mean that they will definitely be in 1.3 - someone still needs to do
>> the implementation. If you really want class based generic views, be
>> like Ben and make it happen -- show us the code!.
>
> Oooh, class-based views.
>
> This is something I've been thinking about a lot, and I'll probably dive into it at the Djangocon.eu sprints. Feel free to make a start Jari. ;)
Regards,
David
Jari Pennanen
> * How to deal with state and self. I have written an instantiator that
> wraps the view class and instantiates a new instance of the class for
it does not need to be imported as one imports the view classes
already.
Then one could do: ``url(r'^path/$', ObjectView.instantiator(),
name="object_view")``
Thats mostly just syntactical sugar though, if it were class method I
would name it as create() or something...
But that is such a small detail at the moment it probably does not
matter.
Luke Plant
> * How to decorate the __call__ method without doing a pointless
> override of __call__ that just calls super so that you have
> something to @decorate on top of. Check out the meta class on line
> 175. This allows you to specify 'decorators = [login_required,
> permission_required("polls.can_vote")]' on your subclass. I showed
> this to Jacob at DjangoSki, and he seemed positive.
are some alternatives which use Plain Old Python:
1) from Python 2.6 we could use class decorators, and we could use the
'old style' MyClass = some_class_decorator(MyClass) until we move to
Python 2.6.
2)
class Foo(View):
__call__ = some_decorator(View.__call__)
3) Add the decorators when you call 'instantiator' (which, by the way,
I would rather call 'make_view' or something more specific). You
would then have, *in views.py*:
3a) my_view = some_decorator(make_view(MyViewClass))
or possibly (though I don't think this adds much):
3b) my_view = make_view(MyViewClass,
decorators=[some_decorator])
This has some significant advantages:
- you don't need to worry about method decorators
- you can re-use MyViewClass with different decorators
- it protects us from changing the URLconf. The URLconf always
has a reference to the 'my_view' function, rather than
MyViewClass. Class-based views then remain an implementation
detail, just as they ought to be. It may well be that
a re-usable app provides some views and might switch
from class-based views to normal functions, and URLconfs
should be insulated from that change.
I don't like the idea of special-casing class based views anywhere,
whether to cope with state or anything else. I think a view should
still be 'a callable that takes a request and returns a response'. If
that means we have to add an extra line to create a view function from
a view class, so be it.
Given that it is going to be possible to use any/all of these whatever
Django provides, I think I'm quite strongly in favour of 3a), and
opposed to adding a metaclass which really doesn't add anything
significant. Metaclasses add complications for people attempting to
understand code, and should be used only when you really need them.
> * How to decorate methods, when the decorator expects the first
> argument to be request, and not self. See line 8. Ideally though,
> Django's decorators would handle this, rather than forcing the use
> of decorate_method_with on to the end users.
with this. All attempts to have decorators that automatically adapt
to functions/methods have failed. See my message here
http://groups.google.com/group/django-developers/msg/f36976f5cfbcbeb3
It has some attachments with test cases that show how our previous
attempt to do this didn't work in some situations.
One thing we could do to make it nicer for the end user is to create
our own 'method' versions of all supplied decorators i.e:
cache_page_m = method_decorator(cache_page)
for every decorator we provide, so that people don't need to do that
themselves.
However, this point may be moot given the discussion above.
Luke
--
"Mistakes: It could be that the purpose of your life is only to
serve as a warning to others." (despair.com)
Luke Plant || http://lukeplant.me.uk/
Nick Fitzgerald
Good points, especially about wrapping decorators *after* instantiation to avoid the whole nasty self issue.
While I personally don't mind meta classes at all, I understand why some people do.
I think you are probably on the right track here.
_Nick_
Reinout van Rees
>
> This is something I've been thinking about a lot, and I'll probably dive into it at the Djangocon.eu sprints.
information about that. Are there actions I need to handle beforehand?
I'm used to Plone sprints and there's usually some "make sure you have
svn access to x and y" stuff best handled *before* the sprint, that's
why I'm asking :-)
Reinout
--
Reinout van Rees - rei...@vanrees.org - http://reinout.vanrees.org
Programmer at http://www.nelen-schuurmans.nl
"Military engineers build missiles. Civil engineers build targets"
Jeremy Dunck
...
> access to x and y" stuff best handled *before* the sprint, that's why I'm
> asking :-)
http://code.djangoproject.com/wiki/Sprints
If you have questions after that, let me know -- I'd be happy to
improve that page.
Harro
sprints too..
Why do all the fun things always happen at the same time..
I guess I could stay the thursday. and then go home on friday.
Ah well.. I'll have till monday to think about it.
On May 14, 4:27 pm, Jeremy Dunck <jdu...@gmail.com> wrote:
Reinout van Rees
> On Fri, May 14, 2010 at 9:12 AM, Reinout van Rees<rei...@vanrees.org> wrote:
> ...
>> I'm used to Plone sprints and there's usually some "make sure you have svn
>> access to x and y" stuff best handled *before* the sprint, that's why I'm
>> asking :-)
>
> This might help:
> http://code.djangoproject.com/wiki/Sprints
wiki sprint page.
> If you have questions after that, let me know -- I'd be happy to
> improve that page.
up an svn trunk django got me frowning a bit: "symlink django trunk into
your system python". Ehm, I'd rather isolate it with virtualenv or
buildout to keep trunk's stuff out of the rest of my django projects.
So: is that just a missing piece of documentation? Or is the virtualenv
kind of isolation rarely done when developing on trunk?
Reinout
Jeremy Dunck
On May 14, 2010, at 4:02 PM, Reinout van Rees <rei...@vanrees.org>
wrote:
> set up an svn trunk django got me frowning a bit: "symlink django
> trunk into your system python". Ehm, I'd rather isolate it with
> virtualenv or buildout to keep trunk's stuff out of the rest of my
> django projects.
>
> So: is that just a missing piece of documentation? Or is the
> virtualenv kind of isolation rarely done when developing on trunk?
made. I should be updated. I use ve on dj dev often.
Luke Plant
> 3) Add the decorators when you call 'instantiator' (which, by the
> way, I would rather call 'make_view' or something more specific).
> You would then have, *in views.py*:
>
> 3a) my_view = some_decorator(make_view(MyViewClass))
>
> or possibly (though I don't think this adds much):
>
> 3b) my_view = make_view(MyViewClass,
> decorators=[some_decorator])
>
I had some more thoughts about this. One problem with applying the
decorators after is if someone is re-using the class (e.g.
subclassing), but forgets to apply some *necessary* decorators when
creating the view from it. One work-around is that 'make_view' checks
the class itself for a 'decorators' attribute, and applies them:
def make_view(viewclass):
def view(request, *args, **kwargs):
obj = viewclass()
return obj(request, *args, **kwargs)
for f in getattr(viewclass, 'decorators', []):
view = f(view)
return view
This would make it harder to forget, but would still allow subclasses
to adjust the decorators that are used by default. It won't help in
the case where people use the class directly in the URLconf, but I
don't think that should be encouraged anyway.
Luke
--
"Oh, look. I appear to be lying at the bottom of a very deep, dark
hole. That seems a familiar concept. What does it remind me of? Ah,
I remember. Life." (Marvin the paranoid android)
Luke Plant || http://lukeplant.me.uk/
David Larlet
We're working on this with Ben during djangocon.eu sprint [1]. Any comment appreciated, still a work in progress though.
Regards,
David
[1] http://github.com/bfirsh/django-class-based-views
Luke Plant
> Hello,
>
> We're working on this with Ben during djangocon.eu sprint [1]. Any
> comment appreciated, still a work in progress though.
Looks cool. I think Jari's idea of a class method to instantiate the
classes would be a great pattern to establish - something like an
'as_view' classmethod on the base class which returns a view function
that uses the class:
@classmethod
def as_view(cls, *initargs, **initkwargs):
def view(*args, **kwargs):
obj = cls(*initargs, **initkwargs)
return obj(*args, **kwargs)
return view
The developer can choose whether to write 'MyClassView.as_view()'
directly into the URLconf, or put a line into views.py like 'my_view =
MyClassView.as_view()'
This will solve the thread safety issue, and doesn't require another
import for a decorator, as Jari pointed out.
We could decide whether to apply decorators inside as_view() or
__call__(). The argument for putting it in 'as_view' is that __call__
does some processing that you might want to skip entirely (e.g. the
get_object() call - if we applied a 'cache_page' decorator, we
wouldn't want the get_object() call to be made at all if there was a
cache hit).
On that point, I think 'GenericView' should be split into 'ClassView'
and 'GenericView'. The get_object() call is probably not a good thing
to have on a base class.
Regards,
David Larlet
We discussed that point with Jacob and it looks like it's easier to modify urlresolvers to create a new instance of the class when we detect it's a class, this should solve the thread safety issue. Do you see any drawback to this approach?
That way we can declare decorators via a decorators' list [1] or directly in the class [2]. I'm not fond of the second way, decorating a decorator, but we can create decorators dedicated to class based views easily that way.
>
> On that point, I think 'GenericView' should be split into 'ClassView'
> and 'GenericView'. The get_object() call is probably not a good thing
> to have on a base class.
Agreed, I'll make that change.
Thanks,
David
[1] http://github.com/bfirsh/django-class-based-views/blob/master/class_based_views/tests/views.py#L24
[2] http://github.com/bfirsh/django-class-based-views/blob/master/class_based_views/tests/views.py#L43
Luke Plant
> > This will solve the thread safety issue, and doesn't require
> > another import for a decorator, as Jari pointed out.
> >
> > We could decide whether to apply decorators inside as_view() or
> > __call__(). The argument for putting it in 'as_view' is that
> > __call__ does some processing that you might want to skip
> > entirely (e.g. the get_object() call - if we applied a
> > 'cache_page' decorator, we wouldn't want the get_object() call
> > to be made at all if there was a cache hit).
>
> We discussed that point with Jacob and it looks like it's easier to
> modify urlresolvers to create a new instance of the class when we
> detect it's a class, this should solve the thread safety issue. Do
> you see any drawback to this approach?
I'm sure that will work. My main reluctance is using isinstance()
when it isn't necessary, and having to change the definition of what a
'view' is when it isn't strictly necessary. (A view is no longer
simply "a callable that takes a request and returns a response"). I
guess this is partly about aesthetics, and I know practicality beats
purity, but:
There might also be the following practical disadvantages when it
comes to the API we encourage.
If the recommended usage is to do:
urlpatterns = ...
(r'something/', MyClassView)
...
then it would be easy to accidentally do `MyClassView()`, and it would
appear to work fine, until you later get thread safety issues. Also,
if you want to pass arguments to the constructor, you would most
naturally do `MyClassView(somearg=something)`, and again it would
appear to work.
If, however, we encouraged `MyClassView.as_view()` from the start, we
can cope with constructor arguments more easily - it changes to
`MyClassView.as_view(somearg=something)`.
(Either way, it would still be possible to pass in MyClassView() and
get thread safety problems, but it's about making mistakes less
natural).
Of course, if we want to use dotted path syntax in URLconfs -
"path.to.MyClassView" - rather than imports, then we would *need* to
have the isinstance() check to be able to cope with it at all. My
response to that is: my preference would be that the use of class
based views should remain an implementation detail, and as far as the
URLconf is concerned, views.py should be exporting a ready-to-use
callable not the class. (The class is exported for the purpose of
other people subclassing, or passing their own constructor arguments,
not for direct use in the URLconf).
There is also the issue of what to do when you need to add a decorator
to someone else's view function. As a developer, I shouldn't have to
know that class based views even *exist*, let alone how to use them,
if I'm just a client - I should *always* be able to say:
from someapp.views import some_view_function
my_view_function = login_required(some_view_function)
But if someapp.views has only exported SomeClassView and not
some_view_function, I now have to do something different.
Given that, for many Django apps, the view functions are part of the
public API that needs to be exported (i.e. hooked into people's
URLconfs or wrapped for re-use), I think it would be good to encourage
practices which will lead to stable and consistent APIs, and so *not*
allow or encourage classes to be used directly in the URLconf.
Jacob Kaplan-Moss
about this IRL at the sprints, and also on the list. I'll try to sum
up where we are now, but if anything isn't clear, ask.
We're currently working towards URLpatterns of the form::
('...', SomeView)
i.e. passing a class directly, which would be instantiated implicitly
then called.
The reason we're trying it this way is thread safety: if a view is a
class, then people *will* store state on self. So if we have single
instances in views like we do now, then we're giving folks enough rope
to hang themselves. By having each request get a fresh instance we
avoid that problem.
The other alternative we discussed was having the base view class do
something with threadlocals to make sure state saved on self is
threadsafe. I vetoed that because I hate threadlocals :)
On Fri, May 28, 2010 at 3:41 PM, Luke Plant <L.Pla...@cantab.net> wrote:
> I'm sure that will work. My main reluctance is using isinstance()
> when it isn't necessary, and having to change the definition of what a
> 'view' is when it isn't strictly necessary. (A view is no longer
> simply "a callable that takes a request and returns a response").
I *am* a bit worried by this -- I'm not sure redefining the definition
of "view" is such a good idea. "A view is a callable or a class with
callable instances" isn't nearly as simple and nice a recommendation.
> If, however, we encouraged `MyClassView.as_view()` from the start, we
> can cope with constructor arguments more easily - it changes to
> `MyClassView.as_view(somearg=something)`.
My only real objection here is that `as_view` is just a bunch of boilerplate::
urlpatterns = patterns('',
('one/', SomeView.as_view()),
('two/', SomeOtherView.as_view()),
('three', YourView.as_view())
)
I just really don't like the way that looks.
As for accepting extra __init__ arguments: I specifically *don't* want
to allow this (at least easily) because I want to encourage
subclassing as the "correct" way of extending views. Yes, this means a
slight loss of flexibility, but it also means that we're forcing
people into a more extensible and fuure-proof way of coding, and
that's a good thing.
> (Either way, it would still be possible to pass in MyClassView() and
> get thread safety problems, but it's about making mistakes less
> natural).
... and either way it's pretty easy to issue a warning if you've
subclassed ``django.views.View`` (my proposed name for the view base
class) and re-used it for multiple requests.
> There is also the issue of what to do when you need to add a decorator
> to someone else's view function.
Again, I want to encourge "subclass it" as the correct answer here.
> Given that, for many Django apps, the view functions are part of the
> public API that needs to be exported (i.e. hooked into people's
> URLconfs or wrapped for re-use), I think it would be good to encourage
> practices which will lead to stable and consistent APIs, and so *not*
> allow or encourage classes to be used directly in the URLconf.
I think we're in agreement here -- we're both trying to avoid view-ish
stuff in the URLconfs (a la the old way we used to do generic views
with dicts in urls.py).
Having played with it for a few years now, though, I'm finding
subclassing is really the best way to make this happen.
So reusable apps instead of exporting functional views export
class-based ones; users who want to re-use them import, subclass, and
extend.
Jacob
Ben Firshman
Storing state on self makes things a heck of a lot easier. We are going to create a "View" and a "ResourceView". A View just renders a template, but a ResourceView has some kind of resource attached to it (model instance, QuerySet, whatever). On View, this would be the get_context() signature (where args and kwargs are from the URL):
def get_context(self, request, *args, **kwargs):
....
When we create ResourceView, we need to pass the resource in here somehow, interfering with args and kwargs:
def get_context(self, request, resource, *args, **kwargs):
....
We would have to redefine all the methods which require the resource. It seems neater to store it on self.
We have solved accidentally instantiating the view by raising an exception if a View instance is called twice. Of course, people who understand the threading issues are free to create their own class-based views.
I like this method because of its transparency. I fear having to explain as_view() in the documentation. Thread problems are hard to understand.
Ben
Luke Plant
> My only real objection here is that `as_view` is just a bunch of
> boilerplate::
>
> urlpatterns = patterns('',
> ('one/', SomeView.as_view()),
> ('two/', SomeOtherView.as_view()),
> ('three', YourView.as_view())
> )
>
> I just really don't like the way that looks.
Agreed. I also think that if you have a mixture of normal views and
class based view, this is ugly:
urlpatterns = patterns('app.views',
('one/', 'some_view_function',
('two/', SomeOtherView),
('three/', YourView)
)
and it would be nicer to spell it:
urlpatterns = patterns('app.views',
('one/', 'some_view_function'),
('two/', 'some_other_view'),
('three/', 'your_view')
)
and have these additional lines in 'app.views':
some_other_view = SomeOtherView.as_view()
your_view = YourView.as_view()
I know that is just moving the boilerplate around, but it is giving a
consistent interface. If some code in a re-usable app moves from
normal views to class based views, they will have to do something like
this *anyway* for backwards compatibility.
But I can see that if subclassing is the default way of re-using, then
exporting these functions gives multiple options about how they should
be re-used, which you are wanting to avoid.
> > There is also the issue of what to do when you need to add a
> > decorator to someone else's view function.
>
> Again, I want to encourge "subclass it" as the correct answer here.
In that case, I guess it would be good to make this hard to do wrong,
by providing helpers that add the decorator to the right end of the
list of decorators.
Ben Firshman
But don't worry, we've changed our minds again! If __call__() creates a copy of self and calls methods on the copy, as far as I can see it solves all our problems. No boilerplate, and it's really hard to make it unsafe thread-wise.
An example of how it could work:
http://github.com/bfirsh/django-class-based-views/blob/master/class_based_views/base.py
Thanks
Ben
Waldemar Kornewald
copying the instance?
Bye,
Waldemar
On May 29, 11:06 pm, Ben Firshman <b...@firshman.co.uk> wrote:
> Luke, you're absolutely right that changing the definition of a view is a bad idea, it just seemed the best solution then.
>
> But don't worry, we've changed our minds again! If __call__() creates a copy of self and calls methods on the copy, as far as I can see it solves all our problems. No boilerplate, and it's really hard to make it unsafe thread-wise.
>
> An example of how it could work:
>
henning....@gmail.com
> Maybe I missed something, but why don't you use __new__ instead of
> copying the instance?
project:
http://djangosnippets.org/snippets/2046/
No __call__ or as_view is needed with this approach.
I can easily replace a function with a class without changing the url
configuration.
Regards,
Henning
Roald
not using a class itself (so basically its __init__ method) as a view.
I'm using something like this in my projects (as a base class):
class View(HttpRequest):
def __init__(self, request, *args, **kwargs):
...
super(View, self).__init__(self.content())
...
You can find it (the commit at time of writing) on
http://github.com/roalddevries/class_based_views/blob/10b5e4016c755164c20126f14870c41dc88b9c03/views.py.
An advantage of this is that url patterns of the form ('...', my_view)
can be user, where my_view is just a class derived from View. I also
think that the solution to redirection (lines 32-39) is pretty
elegant.
I'm looking forward to your comments.
Cheers, Roald
Luke Plant
> Maybe I've missed the reason, or it's just too late to change, but
> why not using a class itself (so basically its __init__ method) as
> a view. I'm using something like this in my projects (as a base
> class):
>
> class View(HttpRequest):
> def __init__(self, request, *args, **kwargs):
> ...
> super(View, self).__init__(self.content())
> ...
>
You mean:
class View(HttpResponse):
...
One reason against this is it makes it harder to re-use other views
from within the View. You are forced to mutate the 'self' instance of
HttpResponse (or else use some nasty hack), rather than being able to
simply return the HttpResponse that might be returned from a utility
function or a sub-view.
Luke Plant
This is an interesting approach. The only downside I can think of at
the moment is that implementing __new__() like that really violates
expectations - Python programmers have a fairly strong expectation
that if you do 'x = SomeClass()', where SomeClass is defined as a
class, then x will be an instance of SomeClass.
Ben Firshman
On 2 Jun 2010, at 22:31, Luke Plant wrote:
> On Tuesday 01 June 2010 11:43:30 henning....@gmail.com wrote:
>> On May 30, 7:24 am, Waldemar Kornewald <wkornew...@gmail.com> wrote:
>>> Maybe I missed something, but why don't you use __new__ instead
>>> of copying the instance?
>>
>> Here is an example where I used __new__ for class based views in my
>> project:
>> http://djangosnippets.org/snippets/2046/
>> No __call__ or as_view is needed with this approach.
>> I can easily replace a function with a class without changing the
>> url configuration.
>
> This is an interesting approach. The only downside I can think of at
> the moment is that implementing __new__() like that really violates
> expectations - Python programmers have a fairly strong expectation
> that if you do 'x = SomeClass()', where SomeClass is defined as a
> class, then x will be an instance of SomeClass.
Yeah, this idea came up at the sprints, but it's a little too magic for my tastes.
Ben
Horst Gutmann
> Luke, you're absolutely right that changing the definition of a view is a bad idea, it just seemed the best solution then.
>
> But don't worry, we've changed our minds again! If __call__() creates a copy of self and calls methods on the copy, as far as I can see it solves all our problems. No boilerplate, and it's really hard to make it unsafe thread-wise.
>
> An example of how it could work:
>
> http://github.com/bfirsh/django-class-based-views/blob/master/class_based_views/base.py
>
> Thanks
>
> Ben
Hi :-)
I'm not really sure about that approach. As with __new__, __call__
comes (at least for me) with a bunch of expectations. For me it
executes and operates on one single instance of an object. Sure, this
could be interpreted as using the instance as a factory for usually
you'd use class methods for something like that, but still.
On the other hand I can definitely see the merit in it since you could
then easily programmatically modify the "view" factory by just
modifying "yet another object".
-- Horst
Waldemar Kornewald
>
> > On May 30, 7:24 am, Waldemar Kornewald <wkornew...@gmail.com> wrote:
> > > Maybe I missed something, but why don't you use __new__ instead
> > > of copying the instance?
>
> > Here is an example where I used __new__ for class based views in my
> > project:
> >http://djangosnippets.org/snippets/2046/
> > No __call__ or as_view is needed with this approach.
> > I can easily replace a function with a class without changing the
> > url configuration.
>
> This is an interesting approach. The only downside I can think of at
> the moment is that implementing __new__() like that really violates
> expectations - Python programmers have a fairly strong expectation
> that if you do 'x = SomeClass()', where SomeClass is defined as a
> class, then x will be an instance of SomeClass.
is not thread-safe, so we'll have to go with a class-based approach.
The only alternative, then, is to extend Django's URL handling code to
automatically instantiate classes, but then calling a class-based view
would require manually instantiating the class. Yes, __new__ does some
magic, but it at least solves all other problems. Kind of reminds me
of the CAP theorem: no magic, easily callable, thread safe. Pick
two. :)
Bye,
Waldemar Kornewald
Roald
> On Wednesday 02 June 2010 16:08:24 Roald wrote:
> > class View(HttpRequest):
> > def __init__(self, request, *args, **kwargs):
> > ...
> > super(View, self).__init__(self.content())
> > ...
>
> You mean:
>
> class View(HttpResponse):
> ...
> One reason against this is it makes it harder to re-use other views
> from within the View.
old function-views?
> You are forced to mutate the 'self' instance of
> HttpResponse (or else use some nasty hack),
from outside.
> rather than being able to
> simply return the HttpResponse that might be returned from a utility
> function or a sub-view.
View.as_view) all have their drawbacks, and personally I think I like
__init__ best:
- the __call__-option requires a different interpretation of url
patterns
- the __new__-option is actually a hack, does unexpected things and
thus is less readable, and is I think more complex that the __init__-
option
- the static/class method option is not really an improvement over
global functions, and leads to url patterns with more 'boiler plate
code'
- the __init__-option is only as complex as any Django programmer
should understand.
But even if you think that using __init__ is more complex than writing
a utility function: no worries. Normally you will use it something
like this:
class myview(View):
template_file = 'myapp/template.html'
@property
def context_dict(self):
return {'authenticated':
self.request.user.is_authenticated()}
... which you can hardly call complicated.
Carl Meyer
> Yeah, this idea came up at the sprints, but it's a little too magic for my tastes.
that magically copies the instance you call it on? That certainly
breaks my expectations.
Django doesn't shy away from "magic" in other areas where it makes for
a usable API. The things that happen to model and form fields
certainly violate the initial expectations of a Python programmer; but
it turns out that most people are happy to write readable models and
don't care what's happening under the hood, and those who do care can
learn a little something about metaclasses. Seems to me that __new__
is a relatively beautiful solution to a thorny problem: it lets people
use class views in all the same ways they use function views while
maintaining thread-safety, it maintains the "callable that returns
HttpResponse" contract, and most people won't ever see or care about
the "magic" that makes it work.
+1 for __new__.
Carl
Ben Firshman
One advantage of using __call__ over __new__ is passing arguments to __init__. That's handy for lazily configuring views:
(r'', DetailView(queryset=Thing.objects.all())
Ben
Alex Gaynor
> You received this message because you are subscribed to the Google Groups "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to django-develop...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
>
>
I thought we said we really wanted to encourage using subclassing to
solve these problems (providing many types of args in urls.py can
cause problems).
Alex
--
"I disapprove of what you say, but I will defend to the death your
right to say it." -- Voltaire
"The people's good is the highest law." -- Cicero
"Code can always be simpler than you think, but never as simple as you
want" -- Me
Waldemar Kornewald
> One advantage of using __call__ over __new__ is passing arguments to __init__. That's handy for lazily configuring views:
>
> (r'', DetailView(queryset=Thing.objects.all())
Why can't you use this instead:
(r'', 'views.DetailView', {'queryset': Thing.object.all()})
Bye,
Waldemar Kornewald
Luke Plant
> > One reason against this is it makes it harder to re-use other
> > views from within the View.
>
> I don't really understand what you mean. Do you mean re-using
> 'good' old function-views?
Yes, or new ones, or any callable that returns an HttpResponse that I
want to return. It is quite a common pattern to have one view wrap
another or *several* others, and also to use standard response objects
in some utility function or 'sub view'. e.g.
def my_view(request, *args):
if some_condition():
return some_view(request, *args)
elif other_condition():
return some_standard_response(request.user)
else:
...
return render_to_response(...)
def some_standard_response(user)
return HttpResponseRedirect(...)
This would be massively uglified, at best, if we make View a subclass
of HttpResponse.
Regards,
Roald
> On Thursday 03 June 2010 08:59:31 Roald wrote:
> > > One reason against this is it makes it harder to re-use other
> > > views from within the View.
> > I don't really understand what you mean. Do you mean re-using
> > 'good' old function-views?
> Yes, or new ones, or any callable that returns an HttpResponse that I
> want to return. It is quite a common pattern to have one view wrap
> another or *several* others, and also to use standard response objects
> in some utility function or 'sub view'. e.g.
>
> def my_view(request, *args):
> if some_condition():
> return some_view(request, *args)
> elif other_condition():
> return some_standard_response(request.user)
> else:
> ...
> return render_to_response(...)
>
> def some_standard_response(user)
> return HttpResponseRedirect(...)
>
> This would be massively uglified, at best, if we make View a subclass
> of HttpResponse.
think I would choose for a simple function, or maybe __new__. On the
other hand: is that a problem? These classes are a solution to a
problem that I have with a lot of my views, but not with all. It's not
necessary to make every view a class.
Roald
view, your 'my_view' should be a base class of 'some_view',
'some_standard_response' etcetera. ABCMeta gives us a way of adding
(abstract) base classes. In Python I think you would best describe
that with something like:
class my_view(HttpResponse):
__metaclass__ = ABCMeta
def __init__(self, *args, **kwargs):
print 'init my_view'
def __new__(cls, *args, **kwargs):
if some_condition():
return some_view(*args, **kwargs)
elif other_condition():
return some_standard_response(*args, **kwargs)
else:
...
return object.__new__(cls)
my_view.register(some_view)
my_view.register(some_standard_response)
This (or something like it) seems to be able to take the best of the
__new__ and __init__ options.
Patryk Zawadzki
> class my_view(HttpResponse):
> __metaclass__ = ABCMeta
>
> def __init__(self, *args, **kwargs):
> print 'init my_view'
>
> def __new__(cls, *args, **kwargs):
> if some_condition():
> return some_view(*args, **kwargs)
> elif other_condition():
> return some_standard_response(*args, **kwargs)
> else:
> ...
> return object.__new__(cls)
>
>
> my_view.register(some_view)
> my_view.register(some_standard_response)
>
>
> This (or something like it) seems to be able to take the best of the
> __new__ and __init__ options.
Uhm... guys,
Maybe something simpler?
---- 8< ----
from threading import local, Thread
class View(object):
_child = local()
def __new__(cls, *args, **kwargs):
existing = hasattr(cls._child, 'instance')
if not existing:
print 'first time in this thread'
cls._child.instance = super(View, cls).__new__(cls, *args, **kwargs)
return cls._child.instance
def __call__(self, foo):
print 'call', id(self), foo
return 'bar'
test = View()
test(1)
test = View()
test(2)
test = View()
test(3)
class TestThread(Thread):
def run(self):
test = View()
test(4)
t = TestThread()
t.setDaemon(True)
t.start()
t.join()
test = View()
test(5)
---- 8< ----
Try it now, tests itself.
--
Patryk Zawadzki
Patryk Zawadzki
> Maybe something simpler?
Actually you might want to have a couple of instances with different
__init__ params:
---- 8< ----
from threading import local, Thread
class View(object):
_child = local()
def __new__(cls, *args, **kwargs):
if not hasattr(cls._child, 'dict'):
cls._child.dict = {}
param_hash = (args, tuple(kwargs.items()))
existing = cls._child.dict.get(param_hash, None)
if not existing:
print 'first time in this thread with args', param_hash
cls._child.dict[param_hash] = super(View, cls).__new__(cls)
return cls._child.dict[param_hash]
def __call__(self, foo):
print 'call', id(self), foo
return 'bar'
--
Patryk Zawadzki
Patryk Zawadzki
The supposed View class is a pure utility class that is close to a
module. It does not represent a state in an application, it takes
input and returns output, none of it should outlive a "__call__" call.
Therefore I see no reason for a proper implementation to not be
thread-safe. Just treat "self" as immutable past "__init__". In other
words, don't treat "self" as a junkyard for temporary variables.
This is WRONG:
class View(object):
def greet(self):
return HttpResponse('Hi %s' % self.request.POST.get('hello', ''))
def __call__(self, request):
self.request = request
return greet(self)
This is CORRECT and not any harder to design/implement:
class View(object):
def greet(self, request):
return HttpResponse('Hi %s' % request.POST.get('hello', ''))
def __call__(self, request):
return greet(self, request)
I really don't think we should add anti-moron filters at the framework
level past documenting "taking care to ensire immutability of view
instances".
A true moron will always outsmart us with creativity. It's perfectly
possible to write thread-unsafe code without using classes:
req = None
def greet(self, request):
global req
req = request
# ...
return HttpResponse('Hi %s' % req.POST.get('hello', ''))
or even:
def greet(self, request):
greet.foo = request
# ...
return HttpResponse('Hi %s' % greet.foo.POST.get('hello', ''))
:)
--
Patryk Zawadzki
Patryk Zawadzki
On Tue, Jun 15, 2010 at 3:38 PM, Patryk Zawadzki <pat...@pld-linux.org> wrote:
> I really don't think we should add anti-moron filters at the framework
> level past documenting "taking care to ensire immutability of view
> instances".
This came out a bit harsh. What I mean is: teach users how to write
thread-safe views (classes or not) and ignore those who refuse to do
it. They either know what they're doing or there is really no point in
forcing the help upon them.
--
Patryk Zawadzki
Nick Fitzgerald
_Nick_
--
Patryk Zawadzki
daonb
> If you don't mind, I'll talk a bit more to myself.
other to get this in 1.3.
Ben has taken the lead of this and published some code. I suggest we
focus on testing and improving Ben's code rather than keep going back
and examine the basic assumptions:
- the url dispatcher will not be changed
- the user will have an thread-safe instance to work with
Actually, the current code is not that sure on the second point,
here's a line from base.py:
view.request = request # FIXME: Maybe remove? Should this be
encouraged?
As I see it, if we're cloning the view, it should be encouraged. I
forked Ben's code and refactored it so that instead of having the
methods pass 'request' around, use self.request. I believe it made the
generic views more readable and it will help users' make their views
cleaner. My fork is at http://github.com/daonb/django-class-based-views
Benny
Patryk Zawadzki
> Actually, the current code is not that sure on the second point,
> here's a line from base.py:
>
> view.request = request # FIXME: Maybe remove? Should this be
> encouraged?
>
> As I see it, if we're cloning the view, it should be encouraged. I
> forked Ben's code and refactored it so that instead of having the
> methods pass 'request' around, use self.request. I believe it made the
> generic views more readable and it will help users' make their views
> cleaner. My fork is at http://github.com/daonb/django-class-based-views
I consider using self to store temporary variables a bad design
practice. You wouldn't do that in C++ or Java. Having said that I
might be biased.
If you really want to do such hacks, use threading.local instead of
writing to self directly:
class View(object):
storage = threading.local()
def __call__(self, request, id):
self.storage.request = request
# ...
Cloning objects on each call to a method is a really really really
really bad idea and puts unnecessary stress on the garbage collector
(actually if any of the methods are careless enough to pass self to an
external function, it might result in a steady leak of non-freeable
memory).
--
Patryk Zawadzki
Ben Firshman
On 16 Jun 2010, at 08:24, daonb wrote:
>
> As I see it, if we're cloning the view, it should be encouraged. I
> forked Ben's code and refactored it so that instead of having the
> methods pass 'request' around, use self.request. I believe it made the
> generic views more readable and it will help users' make their views
> cleaner. My fork is at http://github.com/daonb/django-class-based-views
The request is passed round so methods look like views to decorators. Magically dropping it for decorators seems a bit scary. :/
Ben
Zachary Voase
for Django, which you can find at http://github.com/zacharyvoase/django-clsview
(skip down to 'Examples' to see it in action).
It solves the thread-safety problem, decoration is easy, pointing to
views from the URLconf is trivial, and the doctests are pretty
extensive.
The downside (from what I’ve found so far) is that explaining how it
works, or doing advanced hacking on it, requires a pretty thorough
understanding of the Python object model.
Let me know what you think. FWIW, the source code is in the public
domain.
-- Z
daonb
>
> The request is passed round so methods look like views to decorators. Magically dropping it for decorators seems a bit scary. :/
>
with its user. In our case, Django publishes a contract ensuring the
user that when he inherits from View and overrides any of: GET, POST,
UPDATE, DELETE,get_response, get_content, get_resource, render_html,
get_template, get_context and a few others, the request attribute of
self is current.
I agree stripping the request looks a bit scary, but at least it's
only 5 lines of simple code:-). They way I see it, decorators are
global so they need the request and Django adds it for them. When the
global decorators are done, Django can strip the request and get the
user to trust our contract.
Benny.
Nick Fitzgerald
_Nick_
Roald
> The request is passed round so methods look like views to decorators. Magically dropping it for decorators seems a bit scary. :/
Then you get a request object as the first parameter of methods, and
you can apply decorators to them (untested):
class View(HttpRequest):
def __new__(cls, request, *args, **kwargs):
"""
make sure not to call HttpRequest.__init__ on the request
again,
since it has already been initialized
"""
request.__class__ = cls # or maybe make a copy of
request first
return request(*args, **kwargs)
@login_required
def __call__(request, *args, **kwargs):
return HttpResponse('')
Patryk Zawadzki
> I have forked bfirsh's code as well on
> github: http://github.com/fitzgen/django-class-based-views
> I have changed the code to use __new__ rather than copying self. I have also
> merged in daonb's commit to make request implicit to all methods via
> self.request.
> All feedback is very welcome! :)
Is there any reason why all of you think not passing request around is a gain?
Would you recommend anyone to do:
---- 8< ----
request = None
def my_view():
global request
return HttpResponse('Hi, %s' % (request.user.get_full_name(), ))
def my_other_view():
globl request
return HttpResponse('Hello again, %s' % (request.user.get_full_name(), ))
# ...
---- 8< ----
As I seriously see no difference between this and pushing stuff to self.request.
Request objects are esoteric. They are only meaningful during that
short period of time you spend in the __call__ method. Once you return
a HttpResponse, request is ready to retire. Storing them at the level
objects grants them potentially eternal life for no reason.
Consider this:
---- 8< ----
class AwesomeAdd(object):
def __call__(self, number1, nubmer2):
self.number1 = number1
self.number2 = number2
return self.do_the_math()
def do_the_math(self):
# OMG BBQ, we cleverly avoided passing params around!!1!one
return self.number1 + self.number2
aa = AwesomeAdd()
foo = aa(3, 5)
---- 8< ----
Doesn't that scream "WTF"? :)
--
Patryk Zawadzki
Patryk Zawadzki
> On 16 jun, 15:45, Ben Firshman <b...@firshman.co.uk> wrote:
>> The request is passed round so methods look like views to decorators. Magically dropping it for decorators seems a bit scary. :/
>
> Just brainstorming: couldn't you have View derive from HttpRequest?
Would you seriously recommend people to extend "int" when they want to
implement multiplication? :)
--
Patryk Zawadzki
Waldemar Kornewald
> github: http://github.com/fitzgen/django-class-based-views
> I have changed the code to use __new__ rather than copying self. I have also
> merged in daonb's commit to make request implicit to all methods via
> self.request.
> All feedback is very welcome! :)
The __new__-based approach is just lovely. :) Really, it's much more
elegant than copying self and it's very convenient to be able to
access self.request and the configuration options from anywhere. This
solves a lot of problems when you want to customize a view's behavior.
We really missed this when we used Django's class-based Feed view.
Some minor issues:
The __init__ function isn't called with *args, **kwargs (at least, I
think this was an oversight).
FormView.POST should probably use self.get_content like View.GET does
by default.
Should there be different methods for each request.method, at all?
Often form POST handling code can reuse the GET part if form
validation fails. By splitting everything up in two methods you blow
up the code unnecessarily. BTW, I think the current code would
unnecessarily instantiate the form two times if the form doesn't
validate.
Also, _load_config_values should guarantee that you don't pass
unsupported arguments. This should also work with inheritance.
Bye,
Waldemar Kornewald
Waldemar Kornewald
> On Thu, Jun 17, 2010 at 1:44 AM, Nick Fitzgerald <fit...@gmail.com> wrote:
>> I have forked bfirsh's code as well on
>> github: http://github.com/fitzgen/django-class-based-views
>> I have changed the code to use __new__ rather than copying self. I have also
>> merged in daonb's commit to make request implicit to all methods via
>> self.request.
>> All feedback is very welcome! :)
>
> Is there any reason why all of you think not passing request around is a gain?
>
> Would you recommend anyone to do:
>
> ---- 8< ----
>
> request = None
>
> def my_view():
> global request
> return HttpResponse('Hi, %s' % (request.user.get_full_name(), ))
>
> def my_other_view():
> globl request
> return HttpResponse('Hello again, %s' % (request.user.get_full_name(), ))
>
> # ...
>
> ---- 8< ----
>
> As I seriously see no difference between this and pushing stuff to self.request.
Please take a deeper look at his code. He doesn't use __init__. He
uses __new__, so each request gets his own View instance.
Instead of
aa = AwesomeAdd()
foo = aa(3, 5)
the __new__-based approach allows to do this:
foo = AwesomeAdd(3, 5)
IOW, the "constructor" directly returns an HttpResponse (foo) instead
of an AwesomeAdd instance.
Bye,
Waldemar Kornewald
Patryk Zawadzki
<wkorn...@gmail.com> wrote:
> Please take a deeper look at his code. He doesn't use __init__. He
> uses __new__, so each request gets his own View instance.
>
> Instead of
>
> aa = AwesomeAdd()
> foo = aa(3, 5)
>
> the __new__-based approach allows to do this:
>
> foo = AwesomeAdd(3, 5)
>
> IOW, the "constructor" directly returns an HttpResponse (foo) instead
> of an AwesomeAdd instance.
There is no difference between using __init__ and __new__ as long as
you treat the instance as a junkyard for temporary variables. If done
properly, class-based views do _not_ need a separate instance for each
request.
Here's an example of a thread-safe view that works happily with just
one instance:
---- 8< ----
class MyView(object):
def __init__(self, queryset, template='bar.html'):
self.qs = queryset
self.template = template
def __call__(self, request, id):
instance = get_object_or_404(self.qs, id=id)
return direct_to_template(request, self.template, {'object': instance})
# ...
url(r'^foo/(?P<id>\d+)/', MyView(queryset=Foo.objects.all(),
template='foo.html')),
---- 8< ----
--
Patryk Zawadzki
Roald
> > On 16 jun, 15:45, Ben Firshman <b...@firshman.co.uk> wrote:
> >> The request is passed round so methods look like views to decorators. Magically dropping it for decorators seems a bit scary. :/
>
> > Just brainstorming: couldn't you have View derive from HttpRequest?
>
> Would you seriously recommend people to extend "int" when they want to
> implement multiplication? :)
First: this is just brainstorming. Maybe it inspires someone for a
better solution somehow. As you might know, I (still) favor deriving
from HttpResponse
Second: users will never extend HttpRequest directly, they will extend
View.
Third: It might be not even as weird as it looks on first sight.
There's nothing wrong with 'response = request.get_response()', so why
not just make 'get_response' the 'default function' of a request:
response = request(). I admint that the name View might nog be the
best in this case.
Fourth: still, this is just brainstorming.
Waldemar Kornewald
This is not at all thread-safe because all requests share the same
view instance and thus the same state. Shared state is not thread-safe
(unless you introduce some locking mechanism which is not an option
here). Thread-safety can only be guaranteed if every thread has its
own state. In other words, every request needs his own View instance.
Bye,
Waldemar Kornewald
Tom Evans
A few points:
1) With inheritance, you should be able to say Derived is-a Base, eg
'a Car is-a Vehicle'. A view is NOT a response, however you wrap it
up. A view is a generator that when called with a request, results in
a response, whether it is a class based view, or a function based
view. Does a view have headers or a mime type?
Perhaps you need to start using different names for these classes,
because inferring that 'a view is-a response' just sounds stupid.
2) I extend HttpResponse. In lots of places.
(more to the __new__ approach)
3) One of the purposes (as far as I can see) of having the view as a
class is to allow non volatile application data to be instantiated
outside of the request-response lifecycle.
In the proposed approach, each request is effectively given its own
cloned view object with which to generate the response. Surely this
means this use case is no longer achievable?
If it is, how? (I'm probably missing something!) If it isn't, then
what _is_ the use case for class based views?
Cheers
Tom
