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
codysoyland
the past (see http://codysoyland.com/2010/feb/3/thread-safe-object-oriented-views-django/
and the relevant discussion thread), and this approach solves the
thread-safety and boilerplate-reduction concerns. However, when
implementing this, the one problem I've been fighting is testing. In
order to test methods on a view built this way is to instantiate it
using object.__new__ and explicitly calling __init__ before you can
start to test view methods, and this may not be clear to somebody who
is writing their first class-based view tests.
Increasingly, I've been warming up to the idea of django treating
class-based views a little bit more special: If the view is a subclass
of BaseView, call it's to_response method instead of treating it as a
callable.
The public API would be more clear. You pass in the request object
into __init__ instead of __call__; you don't have __new__ hacks;
testing is a breeze; you don't have to do any factory wrappers a la
http://github.com/toastdriven/django-haystack/commit/e93be1d142372afa1687fb3e35966c1ccec76241
Waldemar Kornewald
> 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.
I'd like to add here that we already discussed the biggest
disadvantage of deriving from HttpResponse:
With this approach you can't easily pass the request to some other view:
def main_view(request, ...):
if ...:
return sub_view(request, ...)
else:
return other_sub_view(request, ...)
This is a common pattern and it can occur somewhere deep within the
view code. The HttpResponse approach doesn't provide an easy solution.
It doesn't even provide an easy solution for returning
HttpResponseRedirect or other HttpResponse subclasses, as you already
identified.
> (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?
It's still possible in exactly the same way as you do it today:
mydata = ...
urlpatterns('', url('...', 'MyView', {'outside_of_lifecycle': mydata}))
Alternatively, you can derive from MyView:
class SubMyView(MyView):
ouside_of_lifecycle = mydata
However, I don't think that this is the use-case of class-based views
(you can achieve the same with function-based views). The whole point
is being able to reuse as much code as possible while allowing to
flexibly modify the view's behavior at a rather fine-grained level.
Take a look at the Feed view in Django (or look at ModelAdmin from the
admin interface if you prefer):
http://docs.djangoproject.com/en/dev/ref/contrib/syndication/
You can customize the query that gets executed and you can customize
every single field that gets displayed in the feed. It's very easy and
flexible and in contrast to function-based views you don't need to
rewrite the whole view from scratch to change a little detail.
Bye,
Waldemar Kornewald
Nick Fitzgerald
It's still possible in exactly the same way as you do it today:mydata = ...
urlpatterns('', url('...', 'MyView', {'outside_of_lifecycle': mydata}))
Alternatively, you can derive from MyView:
class SubMyView(MyView):
ouside_of_lifecycle = mydata
However, I don't think that this is the use-case of class-based views
(you can achieve the same with function-based views). The whole point
is being able to reuse as much code as possible while allowing to
flexibly modify the view's behavior at a rather fine-grained level.
Take a look at the Feed view in Django (or look at ModelAdmin from the
admin interface if you prefer):
http://docs.djangoproject.com/en/dev/ref/contrib/syndication/
You can customize the query that gets executed and you can customize
every single field that gets displayed in the feed. It's very easy and
flexible and in contrast to function-based views you don't need to
rewrite the whole view from scratch to change a little detail.
Roald
> 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.
responses. Personally, I think a view is a pretty abstract concept
('something that turns a request in a response'), and I would prefer
something more graspable.
Patryk Zawadzki
You don't seem to get how threading works.
There is no state in this object past the assignments in __init__.
These are thread-safe as there is only one instance of the object.
Variables in local scope are always thread-safe as reetrancy grants
you a new local scope every time it enters a function.
--
Patryk Zawadzki
Ian Lewis
instance per request (the example will work unless you start adding
stuff to self or overwriting self.qs etc). Only shared state is stored
at the class level and view customization is done at the class
instance level as well. That state shouldn't change.
Attaching all kinds of stuff to self tends to depend on the order of
the methods called and allows for some ugly coding styles. In that
case the slightly strange idea of merging the request and the view by
extending HttpRequest starts to make sense since otherwise you would
have some state on the request object (like the user etc. as it is
now) and some state on the view object.
That said, both extending HttpRequest and using __new__ seem like a
hacks to me. Personally the idea of a view being a callable class
instance or method is simple and flexable enough to handle any use
cases we are talking about. You just have to think of self as the
*view* rather than a thread safe request because that's what it is.
Having a view object per request makes no sense. You should have a
request object per request. It is largely a result of wanting the
convenience of being able to write sloppy code that writes all kinds
of state to self rather than a technical necessity.
Ian
> --
> 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.
>
>
--
=======================================
株式会社ビープラウド イアン・ルイス
〒150-0021
東京都渋谷区恵比寿西2-3-2 NSビル6階
email: ianm...@beproud.jp
TEL:03-6416-9836
FAX:03-6416-9837
http://www.beproud.jp/
=======================================
Patryk Zawadzki
> The example he provided isn't terribly good but you don't need an view
> instance per request (the example will work unless you start adding
> stuff to self or overwriting self.qs etc). Only shared state is stored
> at the class level and view customization is done at the class
> instance level as well. That state shouldn't change.
>
> Attaching all kinds of stuff to self tends to depend on the order of
> the methods called and allows for some ugly coding styles. In that
> case the slightly strange idea of merging the request and the view by
> extending HttpRequest starts to make sense since otherwise you would
> have some state on the request object (like the user etc. as it is
> now) and some state on the view object.
>
> That said, both extending HttpRequest and using __new__ seem like a
> hacks to me. Personally the idea of a view being a callable class
> instance or method is simple and flexable enough to handle any use
> cases we are talking about. You just have to think of self as the
> *view* rather than a thread safe request because that's what it is.
>
> Having a view object per request makes no sense. You should have a
> request object per request. It is largely a result of wanting the
> convenience of being able to write sloppy code that writes all kinds
> of state to self rather than a technical necessity.
You, sir, deserve a beer!
--
Patryk Zawadzki
Waldemar Kornewald
> On Thu, Jun 17, 2010 at 2:08 PM, Waldemar Kornewald
> <wkorn...@gmail.com> wrote:
>> On Thu, Jun 17, 2010 at 1:42 PM, Patryk Zawadzki <pat...@pld-linux.org> wrote:
>>> 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')),
>> 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.
>
> You don't seem to get how threading works.
Let's keep the discussion productive and not talk BS.
> There is no state in this object past the assignments in __init__.
> These are thread-safe as there is only one instance of the object.
The one-instance approach is no more thread-safe than having a global
variable. In your example nothing bad will happen, but once we get to
some real-world reusable views you'll quickly see a need for saving
thread-local state somewhere. We had that problem with Django's Feed
class, for example, where we needed access to the request in one of
the methods and the only workaround was to instantiate a new Feed for
every request and manually inject a _request attribute. The point is
that in a class-based view you normally have lots of methods for
customizing the view's behavior, so either you pass the whole
thread-local state as arguments to every single method (which is
*very* ugly and tedious) or you save that state in self. Also, the
problem with passing lots of arguments around is that it can make
adding more variables to the current state and thus customizability
difficult. There's no way around allowing thread-local state and the
one instance per request approach solves this elegantly.
Bye,
Waldemar Kornewald
Patryk Zawadzki
So the only limitation is that you don't want to pass parameters to methods?
<sarcasm>Good thing self is passed implicitly or classes would be
totally useless!</sarcasm> ;)
More seriously:
Explicit is better than implicit.
Simple is better than complex.
[...]
If the implementation is hard to explain, it's a bad idea.
--
Patryk Zawadzki
Waldemar Kornewald
> On Thu, Jun 17, 2010 at 6:49 PM, Waldemar Kornewald
> <wkorn...@gmail.com> wrote:
>> The one-instance approach is no more thread-safe than having a global
>> variable. In your example nothing bad will happen, but once we get to
>> some real-world reusable views you'll quickly see a need for saving
>> thread-local state somewhere. We had that problem with Django's Feed
>> class, for example, where we needed access to the request in one of
>> the methods and the only workaround was to instantiate a new Feed for
>> every request and manually inject a _request attribute. The point is
>> that in a class-based view you normally have lots of methods for
>> customizing the view's behavior, so either you pass the whole
>> thread-local state as arguments to every single method (which is
>> *very* ugly and tedious) or you save that state in self. Also, the
>> problem with passing lots of arguments around is that it can make
>> adding more variables to the current state and thus customizability
>> difficult. There's no way around allowing thread-local state and the
>> one instance per request approach solves this elegantly.
>
> So the only limitation is that you don't want to pass parameters to methods?
Imagine what the code would look like with parameter passing:
class View(object):
def __call__(self, request, **kwargs):
qs = self.get_queryset(request, **kwargs)
.....
def get_queryset(self, request, **kwargs):
...
Now someone writes a reusable view for S3 file management which only
has one parameter (bucket_name) and publishes the code as an
open-source project:
class S3View(View):
def get_queryset(request, bucket_name, **kwargs):
self.get_bucket(bucket_name)
...
...
Then you want to use his code in your project:
class MyS3View(S3View):
def get_bucket(self, bucket_name):
# oh no! I need the request and the other configuration
paramters here! :(
Now you have a problem because the other developer was too lazy to
pass request and **kwargs to that little "unimportant" get_bucket
method. This happens in practice and our job is to come up with a
design that makes this impossible or very unlikely. If the
one-instance-per-request solution solves this and saves you from
typing **kwargs all over the place then it absolutely is better, hands
down.
Bye,
Waldemar Kornewald
Łukasz Rekucki
> Attaching all kinds of stuff to self tends to depend on the order of
> the methods called and allows for some ugly coding styles. In that
> case the slightly strange idea of merging the request and the view by
> extending HttpRequest starts to make sense since otherwise you would
> have some state on the request object (like the user etc. as it is
> now) and some state on the view object.
view instance=="execution environment". Function-based views are
pretty much the same, but you just don't have to care about the
environment, because it's composed out of your local variables.
>
> That said, both extending HttpRequest and using __new__ seem like a
> hacks to me. Personally the idea of a view being a callable class
> instance or method is simple and flexable enough to handle any use
> cases we are talking about. You just have to think of self as the
> *view* rather than a thread safe request because that's what it is.
I agree about HttpRequest. As Tom pointed out Request is not a View.
It just doesn't fit the inheritance.
As for the __new__, I guess it's just two diffrent approaches and in
the end you get pretty much the same thing. I think of "self" as
"environment" and the class as "view". In one instance approach, the
instance is the "view" and "environment" is in method parameters or
request (is this right?).
Anyway, I think this discussion in on a wrong track. From a Django
user POV, I really don't care how it's implemented. I'm far more
concerned with the API that generic views will provide.
--
Łukasz Rekucki
Waldemar Kornewald
> I've written about using __new__ to generate HttpResponse objects in
> the past (see http://codysoyland.com/2010/feb/3/thread-safe-object-oriented-views-django/
> and the relevant discussion thread), and this approach solves the
> thread-safety and boilerplate-reduction concerns. However, when
> implementing this, the one problem I've been fighting is testing. In
> order to test methods on a view built this way is to instantiate it
> using object.__new__ and explicitly calling __init__ before you can
> start to test view methods, and this may not be clear to somebody who
> is writing their first class-based view tests.
>
> Increasingly, I've been warming up to the idea of django treating
> class-based views a little bit more special: If the view is a subclass
> of BaseView, call it's to_response method instead of treating it as a
> callable.
You have a strong point here. We could at least provide a
new_instance(request, *args, **kwargs) classmethod for unit tests
which automates the __new__ and __init__ calls. Anyway, I'm not sure
which is worth more:
* relatively easy for unit tests (new_instance())
* enforced thread-safety
* no special code in Django's URL routing
vs
* no-brainer for unit tests
* no enforced thread-safety (you can mistakenly create a global view instance)
* special code in Django's URL routing
Bye,
Waldemar Kornewald
Patryk Zawadzki
<wkorn...@gmail.com> wrote:
> Imagine what the code would look like with parameter passing:
>
> class View(object):
> def __call__(self, request, **kwargs):
> qs = self.get_queryset(request, **kwargs)
> .....
>
> def get_queryset(self, request, **kwargs):
> ...
Why would you hardcore the queryset inside the view? Pass the
pre-filtered queryset to the constructor and use get_objects_or_404 to
filter it basing on what is passed to __call__.
Actually as I see it __call__ is not the best idea:
class GenericView(object):
def __init__(self, queryset):
self.qs = queryset
def details(self, request, id):
obj = get_object_or_404(self.qs, id=id)
return direct_to_template(request, 'details.html', {'object': obj})
def list(self, request):
objs = get_objects_or_404(self.qs)
return direct_to_template(request, 'list.html', {'objects': objs})
# ... and so on
> Now someone writes a reusable view for S3 file management which only
> has one parameter (bucket_name) and publishes the code as an
> open-source project:
>
> class S3View(View):
> def get_queryset(request, bucket_name, **kwargs):
> self.get_bucket(bucket_name)
> ...
> ...
Are you sure you need to pass the bucket name each time you _call_ the
view? I'd pass it at construction time instead.
> Then you want to use his code in your project:
>
> class MyS3View(S3View):
> def get_bucket(self, bucket_name):
> # oh no! I need the request and the other configuration
> paramters here! :(
Request sure, but what other configuration parameters? Default
implementation should under no circumstances pass and silently swallow
**kwargs.
> Now you have a problem because the other developer was too lazy to
> pass request and **kwargs to that little "unimportant" get_bucket
> method. This happens in practice and our job is to come up with a
> design that makes this impossible or very unlikely. If the
> one-instance-per-request solution solves this and saves you from
> typing **kwargs all over the place then it absolutely is better, hands
> down.
See above.
--
Patryk Zawadzki
Johannes Dollinger
> The example he provided isn't terribly good but you don't need an view
> instance per request (the example will work unless you start adding
> stuff to self or overwriting self.qs etc). Only shared state is stored
> at the class level and view customization is done at the class
> instance level as well. That state shouldn't change.
>
> Attaching all kinds of stuff to self tends to depend on the order of
> the methods called and allows for some ugly coding styles. In that
> case the slightly strange idea of merging the request and the view by
> extending HttpRequest starts to make sense since otherwise you would
> have some state on the request object (like the user etc. as it is
> now) and some state on the view object.
>
> That said, both extending HttpRequest and using __new__ seem like a
> hacks to me. Personally the idea of a view being a callable class
> instance or method is simple and flexable enough to handle any use
> cases we are talking about. You just have to think of self as the
> *view* rather than a thread safe request because that's what it is.
>
> Having a view object per request makes no sense. You should have a
> request object per request. It is largely a result of wanting the
> convenience of being able to write sloppy code that writes all kinds
> of state to self rather than a technical necessity.
>
> Ian
I'd like to support Ian and Patryk's position: class based views
should not be instantiated for each request - for all the reasons Ian
gave in the quoted mail.
If you want to avoid passing around (lots of) arguments between
methods, you can always store request specific state on the request
instance, which is a common pattern for middleware already.
To prevent the request's dict from becoming too cluttered, simply put
the arguments into `request.view_context` (a dict or similar
container, needs to be painted).
__
Johannes
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.
>
>
So here's an idea, of dubious quality ;) Only pass request around to
methods, and have a requests dictionary on the view (which is
instantiated once) then methods can use that dict to store data in it.
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
Jacob Kaplan-Moss
> So here's an idea, of dubious quality ;)
Okay, folks: at this point the discussion has pretty much descended
into bikeshedding territory. I'm going to read through everything
posted so far and try to post a summary and round-up to help us get
refocused; gimme a few hours to pull that together and then let's try
to reach towards a consensus.
Jacob
Waldemar Kornewald
I've started putting everything into a summary spreadsheet. Feel free
to change/extend it:
https://spreadsheets.google.com/ccc?key=0AnLqunL-SCJJdGhxSVZaQkNCcTlzM2d4OEc5dFRPUUE&hl=en
Bye,
Waldemar Kornewald
Patryk Zawadzki
Some of the approaches also allow for easy grouping of similar views.
For example I'm currently working on an ArchiveView with methods:
* index(self, request)
* details(self, request, year, month, day, id)
* day(self, request, year, month, day)
* week(...)
* month(...)
* year(...)
So you only instantiate one object (telling it which templates to use,
what month format you prefer etc.) and bind its methods to various
URLs.
--
Patryk Zawadzki
Waldemar Kornewald
Any results?
Bye,
Waldemar Kornewald
Roald
> Okay, folks: at this point the discussion has pretty much descended
> into bikeshedding territory.
I just started a new thread on this list, 'ModelView'. It's very
similar to class based generic views; I don't know if it should
actually replace them, or that it's more of a special-case thing (I
think the first). The requesthandler decorator, I think, is relevant
to this thread in either case.
Looking forward to your opinions!
Cheers, Roald