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.
> 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.
On 1 October 2010 07:26, Russell Keith-Magee <russ...@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
(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.
On Fri, 2010-10-01 at 11:12 +0200, Hanne Moa wrote: > On 1 October 2010 07:26, Russell Keith-Magee <russ...@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
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:
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)
> 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.
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):
@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))
> 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.
> -- > You received this message because you are subscribed to the Google Groups "Django developers" group. > To post to this group, send email to django-developers@googlegroups.com. > To unsubscribe from this group, send email to django-developers+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
-- Best regards, Yuri V. Baburov, ICQ# 99934676, Skype: yuri.baburov, MSN: bu...@live.com
> 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:
> 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.
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:
> 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:
>> 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
> -- > You received this message because you are subscribed to the Google Groups "Django developers" group. > To post to this group, send email to django-developers@googlegroups.com. > To unsubscribe from this group, send email to django-developers+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
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.
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.
On Fri, Oct 1, 2010 at 9:53 AM, Vinay Sajip <vinay_sa...@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.
> Regards,
> Vinay Sajip
> -- > You received this message because you are subscribed to the Google Groups "Django developers" group. > To post to this group, send email to django-developers@googlegroups.com. > To unsubscribe from this group, send email to django-developers+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
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
On Fri, Oct 1, 2010 at 6:46 PM, burc...@gmail.com <burc...@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.
> 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):
> @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))
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.
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.
On Fri, Oct 1, 2010 at 6:17 PM, Luke Plant <L.Plant...@cantab.net> wrote: > On Fri, 2010-10-01 at 11:12 +0200, Hanne Moa wrote: >> On 1 October 2010 07:26, Russell Keith-Magee <russ...@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
> 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:
> 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.
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.
On Fri, Oct 1, 2010 at 9:55 AM, Alex Gaynor <alex.gay...@gmail.com> wrote: > On Fri, Oct 1, 2010 at 9:53 AM, Vinay Sajip <vinay_sa...@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.
On Fri, Oct 1, 2010 at 6:16 PM, Johannes Dollinger
<emulb...@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.
<russ...@keith-magee.com> wrote: > On Fri, Oct 1, 2010 at 6:16 PM, Johannes Dollinger > <emulb...@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-developers@googlegroups.com. > To unsubscribe from this group, send email to django-developers+unsubscribe@googlegroups.com. > For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
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
> 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?
On Fri, Oct 1, 2010 at 11:20 PM, Alex Gaynor <alex.gay...@gmail.com> wrote: > On Fri, Oct 1, 2010 at 11:16 AM, Russell Keith-Magee > <russ...@keith-magee.com> wrote: >> On Fri, Oct 1, 2010 at 6:16 PM, Johannes Dollinger >> <emulb...@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.
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.
>> 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
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.
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?
Luke
-- "Despair: It's always darkest just before it goes pitch black." (despair.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.
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...
Luke
-- "Despair: It's always darkest just before it goes pitch black." (despair.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:
> 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?
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
