#6735 -- Class based generic views: call for comment

92 views
Skip to first unread message

Russell Keith-Magee

unread,
Oct 1, 2010, 1:26:12 AM10/1/10
to Django Developers
Hi all,

I've just added a summary of the last thread on class-based views [1].
This summary isn't 100% complete -- any contributions from interested
parties are welcome. Try to keep opinions to a minimum; this page is
about documenting the strengths and weaknesses of various approaches,
not about expressing opinions. In the same way that CSRF [2] and
session-messages [3] have good wiki pages describing the design
considerations, we need to be able to explain to future generations
why class-based views are the way they are.

Based on the discussion on the most recent thread [4], plus in-person
discussions, I'm proposing that we move forward with Ben Firshman and
David Larlet's "__call__ + copy()" approach. Yes, the copy() is a bit
of a weird idiom, but it's not an idiom that users need to be
particularly concerned with; the code "just works" in an unsurprising
way. The other viable option is the __new__ based copy; however, this
approach is also idiomatically surprising (instantiation returns an
object of type other than the object being constructed), and it
doesn't allow for the use of arguments at time of instantiation.

I'd like to try and get this in for 1.3. It's a big feature, which
means it needs to be in by October 18. However, based on the state of
the patch that Ben has prepared, I think this is an achievable goal.

Three things are required to make this happen:

* Documentation. It looks like David Larlet has made a start on this
already, but there's lots more required.
* Conversion of the github project into a patch on trunk.
* Feedback on the API, especially the generic view replacements.

If you have any feedback, drop it into this thread so that we can
integrate the changes. If you are interested in helping out on the
first two points, drop a line here too so that we can coordinate
activity.

I already have one specific piece of API feedback: the current
implementation requires that all view logic is contained in GET(),
POST() etc style views. This is fine, except for the fact that there
are occasions where GET and POST share a lot of common logic. It would
make sense to me to allow for a request() method that is the direct
analog of how we do views right now, wrapping lines 53-59 of base.py
into a request() method. This would allow users to define their own
dispatch mechanisms, or share logic between GET and POST (and any
other http method) as necessary.

[1] http://code.djangoproject.com/wiki/ClassBasedViews
[2] http://code.djangoproject.com/wiki/CsrfProtection
[3] http://code.djangoproject.com/wiki/SessionMessages
[4] http://groups.google.com/group/django-developers/browse_thread/thread/e23d9a5a58fe5891

Yours,
Russ Magee %-)

Santiago Perez

unread,
Oct 1, 2010, 1:58:12 AM10/1/10
to django-d...@googlegroups.com
I already have one specific piece of API feedback: the current
implementation requires that all view logic is contained in GET(),
POST() etc style views. This is fine, except for the fact that there
are occasions where GET and POST share a lot of common logic. It would
make sense to me to allow for a request() method that is the direct
analog of how we do views right now, wrapping lines 53-59 of base.py
into a request() method. This would allow users to define their own
dispatch mechanisms, or share logic between GET and POST (and any
other http method) as necessary.

I'm currently using a very similar approach for class-based views except in my request() implementation I use the request.method and look at self for an attribute with the method name uppercased. if it isn't found an HttpResponseNotAllowed is returned and the list of permited methods is built from checking the view instance for the known methods. Here's an example of the constructor and request implementations: http://gist.github.com/605801 . I have simplified my code for clarity so I may have introduced some bugs.

Hanne Moa

unread,
Oct 1, 2010, 5:12:47 AM10/1/10
to django-d...@googlegroups.com
On 1 October 2010 07:26, Russell Keith-Magee <rus...@keith-magee.com> wrote:
> I already have one specific piece of API feedback: the current
> implementation requires that all view logic is contained in GET(),
> POST() etc style views. This is fine, except for the fact that there
> are occasions where GET and POST share a lot of common logic.

This! is! Python! Any shared logic can be in another method, or
function for that matter, and need not be canonicalized in an API. All
that would be needed is an example in the docs:

class TheView(views.ClassView):

....def _common(self, bla):
........# set up models, forms, other preprocessing here
........# return models/forms or set them on class
........# could be run from __init__ or __new__ or whatever we wind up with

....def GET(self, bla):
........self._common(bla)

....def POST(self, bla):
........self._common(bla)

(Indent-dots to make surer that gmail doesn't meddle with the text.)

Surely class-based views shouldn't disallow methods not named nor
given meaning in an API? Since you needed to explicitly wish for a
specific method, that's how it can be interpreted.


HM

Luke Plant

unread,
Oct 1, 2010, 6:17:10 AM10/1/10
to django-d...@googlegroups.com
On Fri, 2010-10-01 at 11:12 +0200, Hanne Moa wrote:
> On 1 October 2010 07:26, Russell Keith-Magee <rus...@keith-magee.com> wrote:
> > I already have one specific piece of API feedback: the current
> > implementation requires that all view logic is contained in GET(),
> > POST() etc style views. This is fine, except for the fact that there
> > are occasions where GET and POST share a lot of common logic.
>
> This! is! Python! Any shared logic can be in another method, or
> function for that matter, and need not be canonicalized in an API. All
> that would be needed is an example in the docs:
>
> class TheView(views.ClassView):
>
> ....def _common(self, bla):
> ........# set up models, forms, other preprocessing here
> ........# return models/forms or set them on class
> ........# could be run from __init__ or __new__ or whatever we wind up with
>
> ....def GET(self, bla):
> ........self._common(bla)
>
> ....def POST(self, bla):
> ........self._common(bla)

Passing things around between '_common' and GET and POST makes a simple
view much more complex than it needs to be, especially when you have
various local variables that you now have to assign in some way. In the
end you will end up just routing it all to the one method:

....def GET(self, *args, **kwargs):
........return self._common(*args, **kwargs)

....def POST(self, *args, **kwargs):
........return self._common(*args, **kwargs)

This is just 4 lines of annoying boilerplate to undo the dispatching
that was 'helpfully' done for you.

I would definitely support a 'request' method (perhaps call it
'dispatch'?) that allows us to avoid that logic. I suspect that every
view I've created that used a form would be hampered by having dispatch
based on HTTP verb done first.

Luke

--
"Despair: It's always darkest just before it goes pitch black."
(despair.com)

Luke Plant || http://lukeplant.me.uk/

David Larlet

unread,
Oct 1, 2010, 6:42:24 AM10/1/10
to django-d...@googlegroups.com

Russell Keith-Magee a �crit :

> I've just added a summary of the last thread on class-based views [1].
> This summary isn't 100% complete -- any contributions from interested
> parties are welcome. Try to keep opinions to a minimum; this page is
> about documenting the strengths and weaknesses of various approaches,
> not about expressing opinions. In the same way that CSRF [2] and
> session-messages [3] have good wiki pages describing the design
> considerations, we need to be able to explain to future generations
> why class-based views are the way they are.
Thanks for taking that time.

> I'd like to try and get this in for 1.3. It's a big feature, which
> means it needs to be in by October 18. However, based on the state of
> the patch that Ben has prepared, I think this is an achievable goal.

I think too but it needs a lot of feedback from the community.


>
> Three things are required to make this happen:
>
> * Documentation. It looks like David Larlet has made a start on this
> already, but there's lots more required.

Yes, only a start but I'm working on it, any help is really appreciated.


> * Conversion of the github project into a patch on trunk.

Do you see that as a total replacement of actual views.generic or should
we go for a old/new dance?


> * Feedback on the API, especially the generic view replacements.

A first feedback from a fork [1], I think it can be interesting to move
the paginate view too.


>
> I already have one specific piece of API feedback: the current
> implementation requires that all view logic is contained in GET(),
> POST() etc style views. This is fine, except for the fact that there
> are occasions where GET and POST share a lot of common logic. It would
> make sense to me to allow for a request() method that is the direct
> analog of how we do views right now, wrapping lines 53-59 of base.py
> into a request() method. This would allow users to define their own
> dispatch mechanisms, or share logic between GET and POST (and any
> other http method) as necessary.

What do you think of [2], is that what you have in mind? I chose
dispatch, as Luke suggested, because request can be confusing.

Best,
David

[1]
http://github.com/josesoa/django-class-based-views/commit/5f24e25e83f63b44b7049a746713bec0cb16e34a
[2]
http://github.com/bfirsh/django-class-based-views/commit/20b63340bbf2b6152f88b5ee47865689d9fae3a6

bur...@gmail.com

unread,
Oct 1, 2010, 6:46:37 AM10/1/10
to django-d...@googlegroups.com
Hi Russell,

Few more questions I haven't seen answers yet.

How should urls() work for such views? Should they?
How can self.redirect_to() get current app, namespace and function
name to work if one should use reverse() ?
How should request should be passed for self.render()? Implicitly or explicitly?

You've taken GET/POST dispatch as an example of the new approach, but
I suggest you to use this example (sorry for low-quality draft):

class UserView(View):
templates_prefix = ''
users = User.objects.all()
app_instance = 'users'

list_template = '%slist.html'
edit_template = '%sedit.html'

def template_for(page):
return getattr(self, 'list_' % self.templates_prefix)

@property # as we now have >= python 2.4
def urls(self):
return patterns('',
url(r'^$',
self.list,
name='%s:list'),
url(r'^(.+)/$',
self.edit,
name='%s:edit'),
)

def redirect_to(self, request, page):
#how to get full path to the page from attribute page???
return HttpResponseRedirect(request.build_absolute_uri(request)
+ reverse(self, urlconf=request.urlconf) + reverse(page,
urlconf=self.urls))

def list(self, request):
return self.render(self.template_for('list'), {'users': self.users})

def edit(self, request, id):
if not self.users.filter(id=id).exists():
return self.redirect_to(self.list)
return self.render(self.template_for('edit'), {'user':
self.users.get(id=id)})

class CustomersView(View):
list_template = 'generic_list.html'
edit_template = 'my_edit.html'
users = Users.objects.filter(is_staff=False)

url(r'^detail/users/', CustomersView(), name="users", app_name="users"),

> --
> 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.
>
>

--
Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov,
MSN: bu...@live.com

Andrew Godwin

unread,
Oct 1, 2010, 7:25:54 AM10/1/10
to django-d...@googlegroups.com
On 01/10/10 11:17, Luke Plant wrote:
> Passing things around between '_common' and GET and POST makes a simple
> view much more complex than it needs to be, especially when you have
> various local variables that you now have to assign in some way. In the
> end you will end up just routing it all to the one method:
>
> ....def GET(self, *args, **kwargs):
> ........return self._common(*args, **kwargs)
>
> ....def POST(self, *args, **kwargs):
> ........return self._common(*args, **kwargs)
>
> This is just 4 lines of annoying boilerplate to undo the dispatching
> that was 'helpfully' done for you.
>
> I would definitely support a 'request' method (perhaps call it
> 'dispatch'?) that allows us to avoid that logic. I suspect that every
> view I've created that used a form would be hampered by having dispatch
> based on HTTP verb done first

There's nothing stopping Django shipping a base view which just calls a
request method, and then another base view which inherits from that and
does method-based dispatch - we shouldn't add too many classes, else
it'll be a documentation nightmare, but that seems a sensible split.

Andrew

Ben Firshman

unread,
Oct 1, 2010, 8:17:13 AM10/1/10
to django-d...@googlegroups.com
I don't think we should worry about documentation nightmares - we could leave most of it undocumented and just expose the bits which look like the current generic views (and a few base views) as the public API.

I have added a "how to help" section to the wiki page:

http://code.djangoproject.com/wiki/ClassBasedViews

Ben

Johannes Dollinger

unread,
Oct 1, 2010, 6:16:07 AM10/1/10
to django-d...@googlegroups.com
Am 01.10.2010 um 07:26 schrieb Russell Keith-Magee:
> I've just added a summary of the last thread on class-based views [1].
> This summary isn't 100% complete -- any contributions from interested
> parties are welcome. Try to keep opinions to a minimum; this page is
> about documenting the strengths and weaknesses of various approaches,
> not about expressing opinions. In the same way that CSRF [2] and
> session-messages [3] have good wiki pages describing the design
> considerations, we need to be able to explain to future generations
> why class-based views are the way they are.

Could you (or anyone knowledgable) add a section, that explains why each request should have its own view instance?
The thread-safety argument alone does not suffice: if all _request_ state would be stored on request instead of the view, you wouldn't need new instances per request. You could also pass around state explicitly - which admittedly gets messy quickly.
So is this purely about preventing users from shooting themselves in the foot? (hyperbole: Will there be protection from globals in django 1.4?)

> Based on the discussion on the most recent thread [4], plus in-person
> discussions, I'm proposing that we move forward with Ben Firshman and
> David Larlet's "__call__ + copy()" approach. Yes, the copy() is a bit
> of a weird idiom, but it's not an idiom that users need to be
> particularly concerned with; the code "just works" in an unsurprising
> way.

I disagree. If you define a class-based view, create an instance (a singleton) and put it in your urlconf, you wouldn't (or shouldn't) expect it to magically reinstantiate itself everytime it's called. So at least the docs will have to point out the magic prominently.

Regardless of my resentments, thanks for finally creating an official foundation for class based views.

__
Johannes

Vinay Sajip

unread,
Oct 1, 2010, 9:53:32 AM10/1/10
to Django developers
On Oct 1, 11:16 am, Johannes Dollinger <emulb...@googlemail.com>
wrote:
> Could you (or anyone knowledgable) add a section, that explains why each request should have its own view instance?
> The thread-safety argument alone does not suffice: if all _request_ state would be stored on request instead of the view, you wouldn't need new instances per request. You could also pass around state explicitly - which admittedly gets messy quickly.
> So is this purely about preventing users from shooting themselves in the foot? (hyperbole: Will there be protection from globals in django 1.4?)

I think Johannes has a point. The copy-on-call may seem useful at
first sight, but as it's a shallow copy, it may perhaps engender a
false sense of security. So

view_instance.attr = x

will not result in threads stepping on each other's toes, but

view_instance.container_attr.attr = x

surely will?

It seems better to stress thread-safety dos and don'ts in the
documentation.

Regards,

Vinay Sajip

Alex Gaynor

unread,
Oct 1, 2010, 9:55:10 AM10/1/10
to django-d...@googlegroups.com
> --
> 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.
>
>

Without wanting to beat a dead horse, I concur. Fundamentally we need
to have a little faith in your users, the admin has the exact same
pattern of a single instance, and we haven't done anything special
there.

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

Russell Keith-Magee

unread,
Oct 1, 2010, 10:43:38 AM10/1/10
to django-d...@googlegroups.com
On Fri, Oct 1, 2010 at 6:46 PM, bur...@gmail.com <bur...@gmail.com> wrote:
> Hi Russell,
>
> Few more questions I haven't seen answers yet.
>
> How should urls() work for such views? Should they?

Erm... no. A view doesn't have a collection of URLs. A view is a view.
What we're debating here is a way to construct generic views as a
class, rather than as a method with a hideously long argument list.

This isn't a class based view. It's a collection of views, wrapped in
a class. You might be able to call it an app. #6735 isn't about apps -
it's about single views.

Yours,
Russ Magee %-)

Russell Keith-Magee

unread,
Oct 1, 2010, 10:58:40 AM10/1/10
to django-d...@googlegroups.com
On Fri, Oct 1, 2010 at 6:42 PM, David Larlet <lar...@gmail.com> wrote:
>
> Russell Keith-Magee a écrit :

>
>>  * Conversion of the github project into a patch on trunk.
>
> Do you see that as a total replacement of actual views.generic or should we
> go for a old/new dance?

Unless you're proposing to maintain backwards compatibility with the
old APIs, we can't just remove the old ones. This essentially means an
old/new arrangement of some kind is required. I'm not especially hung
up on the name, as long as it's simple and memorable.

>>  * Feedback on the API, especially the generic view replacements.
>
> A first feedback from a fork [1], I think it can be interesting to move the
> paginate view too.

Sounds like an interesting idea.

>> I already have one specific piece of API feedback: the current
>> implementation requires that all view logic is contained in GET(),
>> POST() etc style views. This is fine, except for the fact that there
>> are occasions where GET and POST share a lot of common logic. It would
>> make sense to me to allow for a request() method that is the direct
>> analog of how we do views right now, wrapping lines 53-59 of base.py
>> into a request() method. This would allow users to define their own
>> dispatch mechanisms, or share logic between GET and POST (and any
>> other http method) as necessary.
>
> What do you think of [2], is that what you have in mind? I chose dispatch,
> as Luke suggested, because request can be confusing.

As I said in my response to Luke -- it's not only confusing; it's
impossible due tot he clash with self.request. Beyond that, it's a bit
of a bikeshed. dispatch(), handle(), or anything else simple and
memorable will be fine.

Yours,
Russ Magee %-)

Russell Keith-Magee

unread,
Oct 1, 2010, 10:59:15 AM10/1/10
to django-d...@googlegroups.com

It's been pointed out to me that we can't really call it request()
anyway, because it would clash with self.request. dispatch() is as
good a name as any; handle() would be my only other suggestion.
However, this really is bikeshedding at this point. Whoever paints it
gets to pick the name.

Yours,
Russ Magee %-)

Tobias McNulty

unread,
Oct 1, 2010, 11:14:56 AM10/1/10
to django-d...@googlegroups.com
On Fri, Oct 1, 2010 at 9:55 AM, Alex Gaynor <alex....@gmail.com> wrote:
On Fri, Oct 1, 2010 at 9:53 AM, Vinay Sajip <vinay...@yahoo.co.uk> wrote:
> It seems better to stress thread-safety dos and don'ts in the
> documentation.

Without wanting to beat a dead horse, I concur.  Fundamentally we need
to have a little faith in your users, the admin has the exact same
pattern of a single instance, and we haven't done anything special
there.

+1

The thread safety issues certainly aren't insignificant, but I think they can be solved by a big fat warning in the documentation.  I do prefer this to the potentially false sense of security that copying the object may involve.

Tobias
-- 
Tobias McNulty, Managing Partner
Caktus Consulting Group, LLC
http://www.caktusgroup.com

Russell Keith-Magee

unread,
Oct 1, 2010, 11:16:53 AM10/1/10
to django-d...@googlegroups.com
On Fri, Oct 1, 2010 at 6:16 PM, Johannes Dollinger
<emul...@googlemail.com> wrote:
> Am 01.10.2010 um 07:26 schrieb Russell Keith-Magee:
>> I've just added a summary of the last thread on class-based views [1].
>> This summary isn't 100% complete -- any contributions from interested
>> parties are welcome. Try to keep opinions to a minimum; this page is
>> about documenting the strengths and weaknesses of various approaches,
>> not about expressing opinions. In the same way that CSRF [2] and
>> session-messages [3] have good wiki pages describing the design
>> considerations, we need to be able to explain to future generations
>> why class-based views are the way they are.
>
> Could you (or anyone knowledgable) add a section, that explains why each request should have its own view instance?
> The thread-safety argument alone does not suffice: if all _request_ state would be stored on request instead of the view, you wouldn't need new instances per request. You could also pass around state explicitly - which admittedly gets messy quickly.

The thread-safety argument is more than enough for me.

*If* all requests state was stored on the request, you're correct -
there wouldn't be a need for a class-based view to be instantiated per
request. However, at that point, you're not really using a class --
you're using a collection of methods that share a sub-module
namespace.

> So is this purely about preventing users from shooting themselves in the foot? (hyperbole: Will there be protection from globals in django 1.4?)

"Don't use global variables" is advice that is contained in any
first-year CS textbook.

"Don't use instance variables on a class" isn't advice you're going to
see printed very often. Yet that would be the advice that would be
*required* in order to use stateless class-based views in the way you
describe.

Objects have state. I don't think it's at all unreasonable to suggest
that a instances of a class based view should be able to have state.
And even if we documented the fact that they can't have state, I'm
willing to *guarantee* that people will *try* to make them stateful.

So - we can:
* spend a bunch of time writing infrastructure to prevent people from
putting stateful variables onto their class-based views
* spend a bunch of time answering the same question over and over
again on django-users
* pick an infrastructure that lets people use classes the way they expect.

>> Based on the discussion on the most recent thread [4], plus in-person
>> discussions, I'm proposing that we move forward with Ben Firshman and
>> David Larlet's "__call__ + copy()" approach. Yes, the copy() is a bit
>> of a weird idiom, but it's not an idiom that users need to be
>> particularly concerned with; the code "just works" in an unsurprising
>> way.
>
> I disagree. If you define a class-based view, create an instance (a singleton) and put it in your urlconf, you wouldn't (or shouldn't) expect it to magically reinstantiate itself everytime it's called. So at least the docs will have to point out the magic prominently.

You say "magic", I say an unusual, but otherwise elegant solution for
hiding an annoying implementation detail.

As an end user, you don't have to care about the fact that it's
"magically" instantiated every time it is used. It just behaves like a
class instance, and is thread safe without you having to think about
it.

Yours,
Russ Magee %-)

Alex Gaynor

unread,
Oct 1, 2010, 11:20:26 AM10/1/10
to django-d...@googlegroups.com
On Fri, Oct 1, 2010 at 11:16 AM, Russell Keith-Magee
<rus...@keith-magee.com> wrote:
> On Fri, Oct 1, 2010 at 6:16 PM, Johannes Dollinger
> <emul...@googlemail.com> wrote:
>> Am 01.10.2010 um 07:26 schrieb Russell Keith-Magee:
>>> I've just added a summary of the last thread on class-based views [1].
>>> This summary isn't 100% complete -- any contributions from interested
>>> parties are welcome. Try to keep opinions to a minimum; this page is
>>> about documenting the strengths and weaknesses of various approaches,
>>> not about expressing opinions. In the same way that CSRF [2] and
>>> session-messages [3] have good wiki pages describing the design
>>> considerations, we need to be able to explain to future generations
>>> why class-based views are the way they are.
>>
>> Could you (or anyone knowledgable) add a section, that explains why each request should have its own view instance?
>> The thread-safety argument alone does not suffice: if all _request_ state would be stored on request instead of the view, you wouldn't need new instances per request. You could also pass around state explicitly - which admittedly gets messy quickly.
>
> The thread-safety argument is more than enough for me.
>
> *If* all requests state was stored on the request, you're correct -
> there wouldn't be a need for a class-based view to be instantiated per
> request. However, at that point, you're not really using a class --
> you're using a collection of methods that share a sub-module
> namespace.
>

Not really. The big win from a class-based view is not being able to
store state, you can do that with local variables, it's being able to
override parts of the behavior without needing to rewrite the entire
view, or add 101 parameters.

>> So is this purely about preventing users from shooting themselves in the foot? (hyperbole: Will there be protection from globals in django 1.4?)
>
> "Don't use global variables" is advice that is contained in any
> first-year CS textbook.
>
> "Don't use instance variables on a class" isn't advice you're going to
> see printed very often. Yet that would be the advice that would be
> *required* in order to use stateless class-based views in the way you
> describe.
>
> Objects have state. I don't think it's at all unreasonable to suggest
> that a instances of a class based view should be able to have state.
> And even if we documented the fact that they can't have state, I'm
> willing to *guarantee* that people will *try* to make them stateful.
>
> So - we can:
>  * spend a bunch of time writing infrastructure to prevent people from
> putting stateful variables onto their class-based views
>  * spend a bunch of time answering the same question over and over
> again on django-users
>  * pick an infrastructure that lets people use classes the way they expect.
>

We don't have that documentation for the admin. Either we need it, or
our users are a little bit smarter than we're giving them credit.

>>> Based on the discussion on the most recent thread [4], plus in-person
>>> discussions, I'm proposing that we move forward with Ben Firshman and
>>> David Larlet's "__call__ + copy()" approach. Yes, the copy() is a bit
>>> of a weird idiom, but it's not an idiom that users need to be
>>> particularly concerned with; the code "just works" in an unsurprising
>>> way.
>>
>> I disagree. If you define a class-based view, create an instance (a singleton) and put it in your urlconf, you wouldn't (or shouldn't) expect it to magically reinstantiate itself everytime it's called. So at least the docs will have to point out the magic prominently.
>
> You say "magic", I say an unusual, but otherwise elegant solution for
> hiding an annoying implementation detail.
>
> As an end user, you don't have to care about the fact that it's
> "magically" instantiated every time it is used. It just behaves like a
> class instance, and is thread safe without you having to think about
> it.
>
> Yours,
> Russ Magee %-)
>

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

Alex

Brandon Konkle

unread,
Oct 1, 2010, 11:25:33 AM10/1/10
to django-d...@googlegroups.com
On Oct 1, 2010, at 10:20 AM, Alex Gaynor wrote:

> Not really. The big win from a class-based view is not being able to
> store state, you can do that with local variables, it's being able to
> override parts of the behavior without needing to rewrite the entire
> view, or add 101 parameters.

If you're storing state with local variables, though, don't you end up having to pass 101 parameters around to your methods?

-Brandon

Russell Keith-Magee

unread,
Oct 1, 2010, 11:37:14 AM10/1/10
to django-d...@googlegroups.com

I'm in agreement that this is the gain from class-based views.
However, as soon as you show someone an instance of a class, they're
going to look for the instance attributes -- that's just how classes
work everywhere else in the world.

Unless you can propose a mechanism for physically preventing people
from assigning instance attributes to the class, I simply don't buy
that documentation saying "don't do that" will be sufficient to avoid
confusion.

>>> So is this purely about preventing users from shooting themselves in the foot? (hyperbole: Will there be protection from globals in django 1.4?)
>>
>> "Don't use global variables" is advice that is contained in any
>> first-year CS textbook.
>>
>> "Don't use instance variables on a class" isn't advice you're going to
>> see printed very often. Yet that would be the advice that would be
>> *required* in order to use stateless class-based views in the way you
>> describe.
>>
>> Objects have state. I don't think it's at all unreasonable to suggest
>> that a instances of a class based view should be able to have state.
>> And even if we documented the fact that they can't have state, I'm
>> willing to *guarantee* that people will *try* to make them stateful.
>>
>> So - we can:
>>  * spend a bunch of time writing infrastructure to prevent people from
>> putting stateful variables onto their class-based views
>>  * spend a bunch of time answering the same question over and over
>> again on django-users
>>  * pick an infrastructure that lets people use classes the way they expect.
>>
>
> We don't have that documentation for the admin.  Either we need it, or
> our users are a little bit smarter than we're giving them credit.

Or a third option -- the use case for the admin is different.
ModelAdmin is a pret-a-porter interface to a high level piece of
functionality with a couple of clearly documented extension points.
The API that ModelAdmin exposes doesn't provide any encouragement to
store state on the ModelAdmin instance itself. The fact that
ModelAdmin isn't thread safe in that way isn't documented, because
it's simply never been an issue -- the interface it provides doesn't
encourage you to do things that would cause the thread non-safety to
be exposed.

This contrasts directly with Class-based views: a low-level view
construction kit. Although the interface for the generic class-based
views will be well defined in the same way as the admin, the base View
class isn't. Best practice may well be to avoid state variables, but I
don't think any amount of documentation will stop people from using
them.

Yours,
Russ Magee %-)

Andrew Godwin

unread,
Oct 1, 2010, 11:38:34 AM10/1/10
to django-d...@googlegroups.com

This is a very good point - one of the reasons it's nice to have
self.request is so you don't end up with ridiculous method signatures.
It's also a lot easier to write inflexible methods if you don't have
state on self - if I'm in, say, get_form(), and I want to change the
form I have based on the arguments in the URL (or anything elsewhere in
the class), I can't do that unless someone thought to pass those
variables in initially in the method signature.

Andrew

Luke Plant

unread,
Oct 1, 2010, 12:07:15 PM10/1/10
to django-d...@googlegroups.com
On Fri, 2010-10-01 at 12:16 +0200, Johannes Dollinger wrote:
> Am 01.10.2010 um 07:26 schrieb Russell Keith-Magee:
> > I've just added a summary of the last thread on class-based views
> [1].
> > This summary isn't 100% complete -- any contributions from
> interested
> > parties are welcome. Try to keep opinions to a minimum; this page is
> > about documenting the strengths and weaknesses of various
> approaches,
> > not about expressing opinions. In the same way that CSRF [2] and
> > session-messages [3] have good wiki pages describing the design
> > considerations, we need to be able to explain to future generations
> > why class-based views are the way they are.
>
> Could you (or anyone knowledgable) add a section, that explains why
> each request should have its own view instance?
> The thread-safety argument alone does not suffice: if all _request_
> state would be stored on request instead of the view, you wouldn't
> need new instances per request. You could also pass around state
> explicitly - which admittedly gets messy quickly.
> So is this purely about preventing users from shooting themselves in
> the foot? (hyperbole: Will there be protection from globals in django
> 1.4?)

It's not just about stopping users from shooting themselves in the foot,
it's about helping them to do the right thing easily. Without this kind
of protection, it will be harder for *anyone*, including experienced
developers who are aware of the problem, to do things correctly. It's
like the autoescaping in templates - I *know* that I should use the
'escape' filter, but without autoescaping it is hard for anyone to do it
right all the time.

One alternative that has been suggested is to pass state around
explicitly, which is definitely best practice, but in many cases might
not be possible (e.g. if you are working with someone else's base class,
and in one overridden method you want to set up some objects which ought
to be available to other methods). You could also attach stateful data
to the request object, but attaching random bits of data to the request
is surely just as ugly a solution as the self.copy() call, especially if
you need to avoid names clashes with all the other attributes attached
to request.

With regards to doing a shallow copy, it should be noted that at the
point the copy is made, the only data attached to the object is data
that has been attached in the constructor. It is of course possible that
methods within the view might mutate such data, but it seems more likely
that it will be used as immutable configuration data.

However, I have already seen some example code that might fall foul of
this problem - someone gave the example of storing a queryset on the
object in the __init__. This will get implicitly mutated (the cache is
filled) when it is used the first time. This could lead to frustrating
problems that wouldn't be found in testing, and doing copy() on the
instance won't help.

So, in light of the fact that developers *will* need to be aware of this
issue, I'd like to tentatively suggest an explicit solution which is
nonetheless easy to use and get right. We could have an explicit 'state'
object that is thrown away for every new request, something like this:

class State(object):
pass

class View(object):
def __call__(self, request, *args, **kwargs):
"""
Main entry point for a request-response process.
"""
self.state = State()
self.request = request
self.args = args
self.kwargs = kwargs
resp = self.dispatch(request, *args, **kwargs)
# let any state get GC'd immediately:
del self.state
del self.request
del self.args
del self.kwargs
return resp

We document the issue, warn people not to store state on the instance
itself, but tell them that if they must have stateful data, it should be
attached to self.state instead of self, and they will be OK. They might
still be bitten if they put mutable configuration data into the __init__
and it gets mutated, but we have made it easy to do the right thing -
doing 'self.state.x' is only slightly harder than 'self.x'

Thoughts?

Santiago Perez

unread,
Oct 1, 2010, 12:17:19 PM10/1/10
to django-d...@googlegroups.com
We document the issue, warn people not to store state on the instance
itself, but tell them that if they must have stateful data, it should be
attached to self.state instead of self, and they will be OK.  They might
still be bitten if they put mutable configuration data into the __init__
and it gets mutated, but we have made it easy to do the right thing -
doing 'self.state.x' is only slightly harder than 'self.x'

Thoughts?

I'm not very convinced about this but I'll just throw it out there, how about keeping the same state object in each request instance (assigned at the base __call__ implementation) and overriding getattr and setattr to work with the state object in request (getattr would fallback to regular attributes set during the constructor when no request was present)? Although this would require using threadlocals to access the right request object from getattr and setattr. 

Luke Plant

unread,
Oct 1, 2010, 12:31:59 PM10/1/10
to django-d...@googlegroups.com
Or, more drastically, we could just do this:

class View(object):
def __call__(self, request, *args, **kwargs):

self.request = request
self.args = args
self.kwargs = kwargs
resp = self.dispatch(request, *args, **kwargs)

self.__dict__ = {} # goodbye state!
return resp

This would kill the ability to use __init__ to store anything on the
instance, since it would only work for the first request, and so would
enforce subclassing as the only means of customising behaviour. It would
be more effective at dealing with thread safety concerns than a
'copy()', but breaking __init__() like that is fairly evil. I'm not sure
if this is a serious suggestion or not...

Santiago Perez

unread,
Oct 1, 2010, 1:25:30 PM10/1/10
to django-d...@googlegroups.com
> This would kill the ability to use __init__ to store anything on the
> instance, since it would only work for the first request, and so would
> enforce subclassing as the only means of customising behaviour. It would
> be more effective at dealing with thread safety concerns than a
> 'copy()', but breaking __init__() like that is fairly evil. I'm not sure
> if this is a serious suggestion or not...

if you remove the ability to set state in the constructor and rely
only in subclassing use __new__ instead of copy and all the thread
safety problems are solved.

Perhaps we could do something like that and provide a @classmethod in
View that easily subclasses a View with some default state, for
example:

views.py
------------
MyView(View):
...
urls.py
----------
url(r'^myview$', myapp.views.MyView, name='myview'),
url(r'^myview$', myapp.views.MyView.with_defaults(somevar=somevalue),
name='myview'),

Johannes Dollinger

unread,
Oct 1, 2010, 1:44:19 PM10/1/10
to django-d...@googlegroups.com


If you are willing to limit view state to a dedicated container, you might as well put in on request (e.g. request.view, request.state, request.context, ...) - and store args/kwargs in it as well.
There would be no need to copy or clear self. It could be added automatically for all views (even those that are plain functions).

However, I am -0 on the self.copy() proposal. While I believe it's the wrong design decision, I'd be happy with any decision at this point.
__
Johannes

Vinay Sajip

unread,
Oct 1, 2010, 2:14:21 PM10/1/10
to Django developers

On Oct 1, 4:16 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:

> "Don't use instance variables on a class" isn't advice you're going to
> see printed very often. Yet that would be the advice that would be
> *required* in order to use stateless class-based views in the way you
> describe.
>
> Objects have state. I don't think it's at all unreasonable to suggest
> that a instances of a class based view should be able to have state.
> And even if we documented the fact that they can't have state, I'm
> willing to *guarantee* that people will *try* to make them stateful.
>

To perhaps complicate the issue a little further, I think there's a
distinction between read-only state and mutable state. It seems
reasonable to let view class instances have read-only state (this
might be part and parcel of increased [re]usability) such as template
names, output mimetype configuration etc. This would be quite safe,
ISTM.

For mutable state during request processing, I'd vote for sticking it
all in the request. Then your method signatures need only include the
request - not unreasonable - and if for some reason you couldn't do
that, there's always thread locals for the requests (duck ;-))

Regards,

Vinay Sajip

David P. Novakovic

unread,
Oct 1, 2010, 6:57:52 PM10/1/10
to django-d...@googlegroups.com
My problem with all of this is that it feels like a hell of a lot of
hoopjumping to deal with something that could be solved in the
Resolver.

I may be missing something obvious here, so please tell me if I am..
but couldn't the resolver just check that quacks like something
OOViewish has been passed in and simply branch and init the class
before calling the view in the same way it normally would?

pseudo code follows:

obj = None
func = None
if hasattr(view,something_an_ooview_has): #general ooview instance checking
obj = view()
func = view.dispatch
else:
func = view

response = func(...)
return response


D

David P. Novakovic

unread,
Oct 1, 2010, 6:59:21 PM10/1/10
to django-d...@googlegroups.com
Sorry in my previous email you could simply pass the uninstantiated
class in the url pattern.

url(r'...',MyOOView,...)

Russell Keith-Magee

unread,
Oct 1, 2010, 9:14:37 PM10/1/10
to django-d...@googlegroups.com
On Saturday, October 2, 2010, David P. Novakovic

<davidno...@gmail.com> wrote:
> My problem with all of this is that it feels like a hell of a lot of
> hoopjumping to deal with something that could be solved in the
> Resolver.
>
> I may be missing something obvious here, so please tell me if I am..
> but couldn't the resolver just check that quacks like something
> OOViewish has been passed in and simply branch and init the class
> before calling the view in the same way it normally would?
>
> pseudo code follows:
>
> obj = None
> func = None
> if hasattr(view,something_an_ooview_has): #general ooview instance checking
>    obj = view()
>    func = view.dispatch
> else:
>    func = view
>
> response = func(...)
> return response
>

Sure. Now decorate the view.

Decorating views in the urlconf is a very common and useful pattern,
but it's a pattern that is pretty much impossible using the approach
you describe.

Yours,
Russ Magee %-)

David P. Novakovic

unread,
Oct 1, 2010, 9:26:12 PM10/1/10
to django-d...@googlegroups.com
Yeah, I actually read your modification of the trac page after posting this :\

I guess what I'm getting at is the more general issue of maintaining
the same interface... is it really that important?

If people acknowledge that they are using a different system, then
they can adjust their expectations accordingly.

For example, this specific case of a decorator could be changed
slightly so that the decorator is passed as a parameter to the url
conf, or something like that.

If the goal here is to be completely unsurprising and fit something
into the existing system then I'll let the thread get back on topic :)
I was just considering the possibility of actually providing another
option which is a bit more of a radical change, but also doesn't
involve some quicky ... tweaks..

Russell Keith-Magee

unread,
Oct 1, 2010, 10:00:48 PM10/1/10
to django-d...@googlegroups.com
On Saturday, October 2, 2010, Vinay Sajip <vinay...@yahoo.co.uk> wrote:
>
> On Oct 1, 4:16 pm, Russell Keith-Magee <russ...@keith-magee.com>
> wrote:
>
>> "Don't use instance variables on a class" isn't advice you're going to
>> see printed very often. Yet that would be the advice that would be
>> *required* in order to use stateless class-based views in the way you
>> describe.
>>
>> Objects have state. I don't think it's at all unreasonable to suggest
>> that a instances of a class based view should be able to have state.
>> And even if we documented the fact that they can't have state, I'm
>> willing to *guarantee* that people will *try* to make them stateful.
>>
>
> To perhaps complicate the issue a little further, I think there's a
> distinction between read-only state and mutable state. It seems
> reasonable to let view class instances have read-only state (this
> might be part and parcel of increased [re]usability) such as template
> names, output mimetype configuration etc. This would be quite safe,
> ISTM.

Sure. Now try and explain what constitutes "mutable state" in a way
that makes sense to new Python converts. I don't see how you can get
into that sort of discussion without getting into deep copying, object
referencing, etc.

> For mutable state during request processing, I'd vote for sticking it
> all in the request. Then your method signatures need only include the
> request - not unreasonable - and if for some reason you couldn't do
> that, there's always thread locals for the requests (duck ;-))

We need to make the distinction here between "ideas that see almost
certainly good ideas/potential best practice" and "things that people
will do because they will expect to be able to do them". I cant deny
that assigning state to the request is one way to avoid these
problems, but it doesn't change the fact that when presented with a
class, people will expect that self.foo will work.

To make matters worse, self.foo *will* work... in the single threaded
devserver. Then you go to production, and everything starts breaking
unpredictably because you have a multithreaded environment.

To make it perfectly clear - any solution that proposes to avoid this
issue purelt with documentation is a non-starter IMHO. I don't care
how well written the description is; not being able to assign to self
is a fundamental change from the expectation of how a class behaves in
every other Python context, and it doesn't take much of a crystal ball
to predict that it will rapidly become a FAQ on django-users. When we
can reasonably predict that something is going to become a support
nightmare, that's the first sign that we need to run the other way.

Now, I'm sure the counterargument is going to be that copy() will be
just as error prone and FAQ inducing. My argument to this is:

* when it fails, it will fail during development, not just in production

* the cases where it will fail isn't the default usage case; it's
only going to be an issue when you have a deep data structure that you
are using in a mutable way.

* the cases where it is even *potentially* a problem are an edge
case of common usage. This is only an issue with the arguments passed
to the constuctor of the class-based view. One of the reasons for
moving to class based views is to get away from the monster list of
arguments required by method based generic views in favor of
subclassing. By virtue of the technique we're introducing, we're
limiting the number of people that will be affected by this issue.

I'm a lot more comfortable with the idea of documenting "don't use
complex data structures as class arguments" than I am about
documenting "don't use self on a class". If nothing else, the
reasoning of "a copy is involved, so make __init__ arguments cheap to
copy" will give some protection.

Yours,
Russ Magee %-)

Ian Lewis

unread,
Oct 1, 2010, 10:26:18 PM10/1/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 12:20 AM, Alex Gaynor <alex....@gmail.com> wrote:
> Not really.  The big win from a class-based view is not being able to
> store state, you can do that with local variables, it's being able to
> override parts of the behavior without needing to rewrite the entire
> view, or add 101 parameters.

Agreed. There is already an object to store state per request. It's
the request object. It's there for a reason. Adding data to the
class instance would be like adding data to the view function (like
view_function.data = "spam"). No one would agree that's a good idea.

> We don't have that documentation for the admin.  Either we need it, or
> our users are a little bit smarter than we're giving them credit.

Playing devil's advocate, the admin may not be accessed as much as the
main site, so bugs involving concurrent access may not crop up as much
or be prioritized lower depending on the organization creating the
website.

--
Ian

http://www.ianlewis.org/

David P. Novakovic

unread,
Oct 1, 2010, 10:37:53 PM10/1/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 12:26 PM, Ian Lewis <ianm...@gmail.com> wrote:
> On Sat, Oct 2, 2010 at 12:20 AM, Alex Gaynor <alex....@gmail.com> wrote:
>> Not really.  The big win from a class-based view is not being able to
>> store state, you can do that with local variables, it's being able to
>> override parts of the behavior without needing to rewrite the entire
>> view, or add 101 parameters.
>
> Agreed. There is already an object to store state per request. It's
> the request object. It's there for a reason. Adding data to the
> class instance would be like adding data to the view function (like
> view_function.data = "spam"). No one would agree that's a good idea.
>

By this reasoning, when you add extra helper methods to your class,
which is the reason for OO views, you need to add/use the data those
methods will modify to the request object? That just doesn't make
sense to me at all. It reminds me of the "Feature Envy" code smell..

D

Ian Lewis

unread,
Oct 1, 2010, 10:44:55 PM10/1/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 11:37 AM, David P. Novakovic
<davidno...@gmail.com> wrote:
> By this reasoning, when you add extra helper methods to your class,
> which is the reason for OO views, you need to add/use the data those
> methods will modify to the request object? That just doesn't make
> sense to me at all. It reminds me of the "Feature Envy" code smell..

I'm not sure I understand your comment. Are you complaining about having to pass
the request object around? I think having a helper view add or modify
data on the request
object is more than reasonable. Please also explain to me what you are
meaning by "Feature Envy".

class ViewClass(...):
...
def helper_method(self, request):
request.somedata = "data"
...

I think there might be a nicer way to do this like adding a state
dictionary to the request so that
there was a place to put request state that's not directly on the
request, but that feels like
bikeshedding at this point, if I used the term correctly.

As Alex has said earlier, this is the way the Admin has always worked
and it works ok IMHO so it makes
sense to do something similar with non-admin class based views.

--
Ian

http://www.ianlewis.org/

Carl Meyer

unread,
Oct 2, 2010, 12:56:57 AM10/2/10
to Django developers


On Oct 1, 10:00 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
> Now, I'm sure the counterargument is going to be that copy() will be
> just as error prone and FAQ inducing. My argument to this is:

Yes.

>   * when it fails, it will fail during development, not just in production
>
>   * the cases where it will fail isn't the default usage case; it's
> only going to be an issue when you have a deep data structure that you
> are using in a mutable way.
>
>   * the cases where it is even *potentially* a problem are an edge
> case of common usage.

None of these points seem accurate to me.

The common failure mode for copy-on-call is someone setting self.foo
to anything mutable (a list, dict, or object instance) in their
__init__ method, and then later mutating it during the course of the
view. Initializing state in __init__ to be modified by other methods
is not an edge case, it's a common pattern of class usage in Python,
almost as common as instance state itself. Likewise, mutable data
structures (lists, dicts, objects) are not an edge case, but common
Python idiom.

And this failure will not fail quickly in development. It will work
just fine in development, but will not be thread-safe, and will fail
only under threaded concurrent load.

> I'm a lot more comfortable with the idea of documenting "don't use
> complex data structures as class arguments" than I am about
> documenting "don't use self on a class". If nothing else, the
> reasoning of "a copy is involved, so make __init__ arguments cheap to
> copy" will give some protection.

> Sure. Now try and explain what constitutes "mutable state" in a way
> that makes sense to new Python converts. I don't see how you can get
> into that sort of discussion without getting into deep copying, object
> referencing, etc.

I don't understand how these two paragraphs are internally consistent.
I agree that explaining the intricacies of mutable vs immutable
objects to beginners is impractical. So how then is it reasonable to
document "you can set instance attributes in __init__ as long as they
are immutable," which is what would have to be documented in the copy-
on-call case?

ISTM that of the thus-far proposed options for "preventing" un-thread-
safe use of class-based views without changing the view contract, the
only one that will fail fast in development is the proposal to make
setting state on self raise an error or warning.

Carl

Russell Keith-Magee

unread,
Oct 2, 2010, 4:34:51 AM10/2/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 12:56 PM, Carl Meyer <carl.j...@gmail.com> wrote:
>
>
> On Oct 1, 10:00 pm, Russell Keith-Magee <russ...@keith-magee.com>
> wrote:
>> Now, I'm sure the counterargument is going to be that copy() will be
>> just as error prone and FAQ inducing. My argument to this is:
>
> Yes.
>
>>   * when it fails, it will fail during development, not just in production
>>
>>   * the cases where it will fail isn't the default usage case; it's
>> only going to be an issue when you have a deep data structure that you
>> are using in a mutable way.
>>
>>   * the cases where it is even *potentially* a problem are an edge
>> case of common usage.
>
> None of these points seem accurate to me.
>
> The common failure mode for copy-on-call is someone setting self.foo
> to anything mutable (a list, dict, or object instance) in their
> __init__ method, and then later mutating it during the course of the
> view. Initializing state in __init__ to be modified by other methods
> is not an edge case, it's a common pattern of class usage in Python,
> almost as common as instance state itself. Likewise, mutable data
> structures (lists, dicts, objects) are not an edge case, but common
> Python idiom.
>
> And this failure will not fail quickly in development. It will work
> just fine in development, but will not be thread-safe, and will fail
> only under threaded concurrent load.

I can't argue with the fact that setting variables in __init__() is a
common idiom in Python generally, and this is certainly a weakness of
copy on call that will fail in non-thread safe ways.

My counterclaim is that the move to class-based views is largely
driven by a desire to discourage the use of arguments in this way. We
want to discourage people from using arguments to class-based views,
and use subclassing instead.

Given that we're discouraging people from using arguments to
__init__(), and that it *will* work for arguments with "simple" data
types, I consider it *marginally* more acceptable than giving users a
class-like structure that straight out won't work like any other class
they've ever used.

Lets be clear here - I don't particularly like *any* of the options on
the table so far. However, my opinion is that copy on call is the
option that will provide the least surprising behavior for the most
common use cases -- having a subset of arguments to __init__ being
unsafe is less surprising to me that having *all* attributes on self
either unsafe, or actively prohibited.

>> I'm a lot more comfortable with the idea of documenting "don't use
>> complex data structures as class arguments" than I am about
>> documenting "don't use self on a class". If nothing else, the
>> reasoning of "a copy is involved, so make __init__ arguments cheap to
>> copy" will give some protection.
>
>> Sure. Now try and explain what constitutes "mutable state" in a way
>> that makes sense to new Python converts. I don't see how you can get
>> into that sort of discussion without getting into deep copying, object
>> referencing, etc.
>
> I don't understand how these two paragraphs are internally consistent.
> I agree that explaining the intricacies of mutable vs immutable
> objects to beginners is impractical. So how then is it reasonable to
> document "you can set instance attributes in __init__ as long as they
> are immutable," which is what would have to be documented in the copy-
> on-call case?

Agreed, there isn't much distinction. I suppose it's a matter of
degrees. A simple "avoid lists and dicts as __init__ args" is an
easier paragraph than something that tries to complicate matters by
talking about readonly state and writable state, and mutable data an
immutable data. The core of the matter isn't actually that different -
it's just that you don't need all the extra framing complicating
matters.

> ISTM that of the thus-far proposed options for "preventing" un-thread-
> safe use of class-based views without changing the view contract, the
> only one that will fail fast in development is the proposal to make
> setting state on self raise an error or warning.

Another option would be to use copy-on-call, but raise an error if
they provide any arguments to __init__(). This would be annoying and
counter to Python idiom, but it strikes me as less fundamentally
counterintuitive than prohibiting *any* use of self to store state.

We could even wrap the "no args to __init__" error check in a method
that enables it to be overridden and silenced in a subclass; that way,
introducing the potentially un-threadsafe behavior would need to be an
active action on the part of the developer. Then we can document the
"feature" up the wahoo -- "If you *really* need arguments to your
class-based view's constructor, you can get them, but here's the
caveats... if you ignore these caveats, here's what will happen"

Yours,
Russ Magee %-)

Łukasz Rekucki

unread,
Oct 2, 2010, 5:21:34 AM10/2/10
to django-d...@googlegroups.com
On 2 October 2010 10:34, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>
> Another option would be to use copy-on-call, but raise an error if
> they provide any arguments to __init__(). This would be annoying and
> counter to Python idiom, but it strikes me as less fundamentally
> counterintuitive than prohibiting *any* use of self to store state.
>
> We could even wrap the "no args to __init__" error check in a method
> that enables it to be overridden and silenced in a subclass; that way,
> introducing the potentially un-threadsafe behavior would need to be an
> active action on the part of the developer. Then we can document the
> "feature" up the wahoo -- "If you *really* need arguments to your
> class-based view's constructor, you can get them, but here's the
> caveats... if you ignore these caveats, here's what will happen"

Now you lost me. If we are discouraging people from using arguments to
__init__, why does the page list "You can't use arguments to init() to
instantiate a class view" as a drawback of the __new__ based approach
?

Anyway, using mutable arguments to view's __init__ is pretty much the
same as using mutable objects as default values for function
arguments. Doing:

def myview(request, options, defaults={'foo': 'bar'}):
defaults.update(options)
.....

will fail even with only one thread (this is a quite common mistake of
novice python programmers). So will using a dictionary as an argument
to __init__. The difference is, that it's easy to explain the function
case without going into much details. In case of __init__ arguments I
can't seem to find a good explanation not involving phrase "global
variable" which I prefer not to use.

My proposal it to drop __init__ args (which actually reduces the
copy/call to the new/call approach). If you still want to set some
options in your urls.py and 2 lines to make a subclass is too much of
boilerplate, here's a friendly helper (you can make it a method of
view):

def with_args(view_cls, **kwargs):
return type(view_cls.__name__, (view_cls,), kwargs)

# in urls.py:

(r'^somepath', with_args(MyView, option=False)),
# or even
(r'^somepath', MyView.with(option=False)),

--
Łukasz Rekucki

Patryk Zawadzki

unread,
Oct 2, 2010, 6:08:57 AM10/2/10
to django-d...@googlegroups.com
On Fri, Oct 1, 2010 at 3:55 PM, Alex Gaynor <alex....@gmail.com> wrote:
> On Fri, Oct 1, 2010 at 9:53 AM, Vinay Sajip <vinay...@yahoo.co.uk> wrote:
>> On Oct 1, 11:16 am, Johannes Dollinger <emulb...@googlemail.com>

>> wrote:
>>> Could you (or anyone knowledgable) add a section, that explains why each request should have its own view instance?
>>> The thread-safety argument alone does not suffice: if all _request_ state would be stored on request instead of the view, you wouldn't need new instances per request. You could also pass around state explicitly - which admittedly gets messy quickly.
>>> So is this purely about preventing users from shooting themselves in the foot? (hyperbole: Will there be protection from globals in django 1.4?)
>> I think Johannes has a point. The copy-on-call may seem useful at
>> first sight, but as it's a shallow copy, it may perhaps engender a
>> false sense of security. So
>>
>> view_instance.attr = x
>>
>> will not result in threads stepping on each other's toes, but
>>
>> view_instance.container_attr.attr = x
>>
>> surely will?

>>
>> It seems better to stress thread-safety dos and don'ts in the
>> documentation.
> Without wanting to beat a dead horse, I concur.  Fundamentally we need
> to have a little faith in your users, the admin has the exact same
> pattern of a single instance, and we haven't done anything special
> there.

I second that view. We should tell people how to write thread-safe
code, not try to work around broken solutions. Thread-safe views are
easy. Just don't write to self once you're out of __init__.
Workarounds are not trivial, can make your life hard in wonderful ways
(like "why is __new__ called twice?") and intentionally break best
practices. I'd really prefer to teach people not to stab themselves
instead of forcing everyone to wear a shiny set of armor.

--
Patryk Zawadzki

Russell Keith-Magee

unread,
Oct 2, 2010, 6:32:45 AM10/2/10
to django-d...@googlegroups.com
2010/10/2 Łukasz Rekucki <lrek...@gmail.com>:

> On 2 October 2010 10:34, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>>
>> Another option would be to use copy-on-call, but raise an error if
>> they provide any arguments to __init__(). This would be annoying and
>> counter to Python idiom, but it strikes me as less fundamentally
>> counterintuitive than prohibiting *any* use of self to store state.
>>
>> We could even wrap the "no args to __init__" error check in a method
>> that enables it to be overridden and silenced in a subclass; that way,
>> introducing the potentially un-threadsafe behavior would need to be an
>> active action on the part of the developer. Then we can document the
>> "feature" up the wahoo -- "If you *really* need arguments to your
>> class-based view's constructor, you can get them, but here's the
>> caveats... if you ignore these caveats, here's what will happen"
>
> Now you lost me. If we are discouraging people from using arguments to
> __init__, why does the page list "You can't use arguments to init() to
> instantiate a class view" as a drawback of the __new__ based approach
> ?

The difference is that __new__ doesn't *ever* allow for initialization
arguments -- there is no way to pass an argument in. An "arguments
disabled by default" __init__+copy implementation allows arguments as
an opt-in.

There's also the wierd behavior that __new__ requires. Consider:

x = MyView()

What is x? If you use the __init__+copy approach, x is an instance of
MyView. If you use __new__, x is a HttpResponse. It's a minor thing,
to be sure, but this is a bit of a code smell to me.

> Anyway, using mutable arguments to view's __init__ is pretty much the
> same as using mutable objects as default values for function
> arguments. Doing:

Yes - but that's something that is well known as a Python gotcha. The
same restriction doesn't usually apply to just passing values to a
constructor.

> def myview(request, options, defaults={'foo': 'bar'}):
>    defaults.update(options)
>    .....
>
> will fail even with only one thread (this is a quite common mistake of
> novice python programmers). So will using a dictionary as an argument
> to __init__. The difference is, that it's easy to explain the function
> case without going into much details. In case of __init__ arguments I
> can't seem to find a good explanation not involving phrase "global
> variable" which I prefer not to use.
>
> My proposal it to drop  __init__ args (which actually reduces the
> copy/call to the new/call approach). If you still want to set some
> options in your urls.py and 2 lines to make a subclass is too much of
> boilerplate, here's a friendly helper (you can make it a method of
> view):
>
> def with_args(view_cls, **kwargs):
>    return type(view_cls.__name__, (view_cls,), kwargs)
>
> # in urls.py:
>
>  (r'^somepath', with_args(MyView, option=False)),
>  # or even
>  (r'^somepath', MyView.with(option=False)),

I can't deny this would work. It just doesn't feel very Pythonic to me.

Yours,
Russ Magee %-)

Russell Keith-Magee

unread,
Oct 2, 2010, 6:49:25 AM10/2/10
to django-d...@googlegroups.com

Opinion noted, but as I've said before, I fundamentally disagree.
The argument here isn't whether it's easy to write a thread-safe view.
It's whether you can accidentally write a thread unsafe view by
following the practices you have used consistently in every other
piece of code you have written.

Python classes have state. People *will* assign variables to self,
because that's something they have done with every other Python class
in existence. And as soon as their code hits production, their code
will break, in unpredictable ways. Claiming that users should
RTFM/should know better isn't an acceptable excuse in this case, IMHO.
When an API will break when you use it in the obvious way, that isn't
something you can hide in documentation and say "we told you so".

Yours,
Russ Magee %-)

Patryk Zawadzki

unread,
Oct 2, 2010, 7:05:52 AM10/2/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 12:49 PM, Russell Keith-Magee
<rus...@keith-magee.com> wrote:
> Python classes have state. People *will* assign variables to self,
> because that's something they have done with every other Python class
> in existence. And as soon as their code hits production, their code
> will break, in unpredictable ways. Claiming that users should
> RTFM/should know better isn't an acceptable excuse in this case, IMHO.
> When an API will break when you use it in the obvious way, that isn't
> something you can hide in documentation and say "we told you so".

Classes that represent real objects have state. Like cars have color,
make and registration number, your models have attributes that
differentiate them from other objects of same class.

"Utility" classes (usually singletons) that represent a function
(service) should not have state.

Imagine walking into a bank and talking to a clerk. She asks for your
name and purpose. You say you want to withrdraw some money. She then
walks up to a giant whiteboard, wipes it and writes your name and your
bank account number there. Then she yells "Joooeeeey" and walks away.
But until Joey appears to give you money, you see another clerk walk
up to the whiteboard, wipe your data and replace it with John Doe. She
then proceeds to exclaim "Joooeeeey" and walks away. That's what a
stateful view is :)

--
Patryk Zawadzki

Patryk Zawadzki

unread,
Oct 2, 2010, 7:06:30 AM10/2/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 1:05 PM, Patryk Zawadzki <pat...@pld-linux.org> wrote:
> But until Joey appears to give you money (...)

s/until/before/

--
Patryk Zawadzki

Russell Keith-Magee

unread,
Oct 2, 2010, 7:40:53 AM10/2/10
to django-d...@googlegroups.com
On Sat, Oct 2, 2010 at 7:05 PM, Patryk Zawadzki <pat...@pld-linux.org> wrote:
> On Sat, Oct 2, 2010 at 12:49 PM, Russell Keith-Magee
> <rus...@keith-magee.com> wrote:
>> Python classes have state. People *will* assign variables to self,
>> because that's something they have done with every other Python class
>> in existence. And as soon as their code hits production, their code
>> will break, in unpredictable ways. Claiming that users should
>> RTFM/should know better isn't an acceptable excuse in this case, IMHO.
>> When an API will break when you use it in the obvious way, that isn't
>> something you can hide in documentation and say "we told you so".
>
> Classes that represent real objects have state. Like cars have color,
> make and registration number, your models have attributes that
> differentiate them from other objects of same class.
>
> "Utility" classes (usually singletons) that represent a function
> (service) should not have state.