#6735 -- Class based generic views: call for comment
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.
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
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.
Hanne Moa
> 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
> 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
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.
> 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
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
> 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
I have added a "how to help" section to the wiki page:
http://code.djangoproject.com/wiki/ClassBasedViews
Ben
Johannes Dollinger
> 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
wrote:
> 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?)
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
> 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
> 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
>
> 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
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
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
<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
<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
> 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
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
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
> 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
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 Plant
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
> 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
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
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.
>
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
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
class in the url pattern.
url(r'...',MyOOView,...)
Russell Keith-Magee
<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
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
>
> 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
> 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
David P. Novakovic
> 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
<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
Carl Meyer
On Oct 1, 10:00 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
> 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.
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.
> 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 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
>
>
> 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
>
> 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
> 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?)
>> 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
> 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
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
<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
> But until Joey appears to give you money (...)
s/until/before/
--
Patryk Zawadzki
Russell Keith-Magee
> 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.
Ok, now show me keyword in Python that identifies a class as a
"utility class". Or the page in the Python documentation where it
talks about "utility classes" and how you identify them and use them
properly.
You can't, because Python doesn't make that distinction. Python has
classes. Class instances have state. Period. People certainly write
classes that are stateless. But providing a base API based around
classes, encouraging people to subclass and add their own behavior,
and expecting *documentation* to prevent them from using self is
complete folly IMHO.
An API that depends on an unenforced contract that you *don't* perform
an action that is otherwise completely normal and accepted behavior
is, in my opinion, prima facie broken. It rates somewhere between
either a 3 and a -4 on Rusty Russell's API design scale [1][2],
depending on how generous you're being. By my reckoning, the
"init-disabled call+copy" rates somewhere in the 5-7 range.
[1] http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html
[2] http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
> 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 :)
Analogies only work when they:
1) make sense
2) are actually analogous.
Neither of these appear to be the case here. The best way I can "fix"
this analogy is to point out that every time you're walking into the
bank, a completely new bank with a clean whiteboard is constructed for
you, and you're the only patron who is allowed to use the bank before
it is demolished.
Yours,
Russ Magee %-)
Carl Meyer
On Oct 2, 4:34 am, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
> 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.
class Foo(View):
def __init__(self):
self.bar = []
self.baz = {}
I can't count the number of times I've done this; can you? This is an
outright failure mode for copy-on-call; no __init__ arguments are
involved.
(I was wrong in my earlier message; this can fail in single-threaded
development in consecutive requests. However, the failure is still
subtle, and depending on the details of what you're doing in the view,
may only manifest under certain circumstances. It still falls firmly
in the category of a confusing and non-explicit failure.)
> 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.
you are representative of Django users in that :-) It is far more
surprising for me for such a low-level API to be making such odd fine-
grained distinctions about whether I can safely assign mutable or
immutable things to my instance in my __init__ method.
The issue is not only the frequency of failure, but how explicit/clear
it is. The failure here is so obscure and difficult to track down, it
is likely to generate an outsize support burden. In contrast, raising
an error on assigning to self can be completely explicit; no hours of
painful and confusing debugging necessary.
Let's also be clear to distinguish "just document that you don't store
state on self" vs "raise an error if you set an attribute on self."
These are proposals with very different characteristics in terms of
likely confusion and support burden.
> 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.
or dicts (or objects, such as QuerySets) as instance attributes in
self (again, whether it's an argument or not is irrelevant), you will
have no choice but to "complicate matters" by talking about mutable vs
immutable objects. By contrast, no such explanation is required if the
rule is "no attributes on self." The explanation there, even after
several levels of "why," is still a simple one-liner: a single view
instance is reused to serve multiple requests, so you can't put
request-specific stuff on it.
> 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
checked is any assignment to self that takes place in __init__. How do
you propose to check that?
Carl
Vince Veselosky
respond until now. I will endeavor to keep it brief as this thread has
grown quite long!
+1 on using a single dispatch() method rather than hard-coding HTTP
method dispatch. As was pointed out, there are many possible dispatch
patterns to fit different applications, and they can be implemented in
subclasses.
Regarding thread safety, I fear that there may be two separate
concerns being conflated in the conversation. When I hear "thread
safety" I think "preventing accidental sharing of data between two
requests running at the same time". In an HTTP application there is a
second concern that I might call "HTTP state safety" which is
"preventing accidental sharing of data between two requests running at
different times".
HTTP is designed to be a stateless protocol, and some of the most
insidious and expensive bugs to find and fix are the ones where
request state *accidentally* leaks from one request to a subsequent
request. For this reason, any good web development framework should
make it *very hard* to leak state between requests. (If you *want* to
share state between requests, Django provides obvious ways for you to
do that already: Models and Sessions.)
We seem to have two competing concerns: the desire to pass arguments
to the view constructor (which argues for a single instance), and the
natural tendency for developers to store request state on the view
instance (which argues for instance-per-request).
This sounds like a job for a Factory Class, doesn't it? What if you
instantiate a ViewFactory in your urls.py, passing it all the
arguments you want to initialize your view instance. At request time,
the ViewFactory instance creates a new View instance, initializing it
with the arguments it stored. It then becomes safe for View instances
to store request state, but they are still initialized as desired. Is
this safer than a shallow copy of a live instance?
Discuss.
Luke Plant
> 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
>
Please ignore this suggestion, and my previous one, if you didn't
already; I don't know what I was thinking, because they don't address
the multi-threaded problem at all.
Luke
--
"DO NOT DISTURB. I'm disturbed enough already."
Luke Plant || http://lukeplant.me.uk/
André Eriksson
On Oct 2, 12:32 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
>
> 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.
return a HttpResponse to me. If this little peculiarness can be gotten
around, I think __new__ can be used to allow for quite an elegant
solution if you ask me. While reading this thread arguments against
this solution are that you either can't use decorators, or that you
cannot pass in arguments to __init__.
First of all, consider the following implementation:
class BaseView(object):
def __new__(cls, *args, **kwargs):
obj = super(BaseView, cls).__new__(cls)
# Call __init__ manually since it will not be called
# after this method (since __new__ returns a HttpResponse
# and not a BaseView subclass).
obj.__init__(*args, **kwargs)
return obj.view() # Or other name
class MyView(BaseView):
def __init__(self, request):
self.request = request
def view(self):
return HttpResponse(self.request.path)
The first argument, that you cannot use decorators, is not true.
Wrapping MyView in login_required() works just fine.
The second argument, that you cannot pass in initialization arguments,
I'm not sure about. It can either be interpreted as "__init__ is not
called with the view arguments like other views", to which I
demonstrated above that it indeed is. If it is to be interpreted as
"you cannot customize your view by calling __init__ with different
arguments for each view consisting of the same class", then yes,
that's true. However, I don't think that's a big problem since the
main focus is on subclassing as customization. It's also not a feature
present in regular, function-based views, and is mainly gotten around
by decorators (which work just fine with the method above).
Forgive me if the solution above has some flaws I can't see that have
already brought up, but to me it seems to have some good qualities.
Best regards,
André Eriksson
George Sakkis
wrote:
>
thread, the summary saved me a whole lot of 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.
> Lets be clear here - I don't particularly like *any* of the options on
> the table so far.
and the general lack of consensus make me a bit uneasy. I'd rather see
the feature postponed to 1.4 or the October 18 date moved later than
ending up with a poor solution. With the backwards compatibility
policy being what it is, design mistakes become very costly.
Having said that, I'm in favour of the first approach mentioned in the
wiki (store state on request) and I'm surprised it doesn't seem to
have any traction in this thread. The only argument against is "it's
unusual to have a class where you can't store state on self" but even
that can be overcome with an appropriate __setattr__/__getattr__ that
access self.request.state under the hood. Unless someone tries
self.__dict__[attr], it will look as if the state is stored on the
view although it's actually stored in the request (or better
request.state to avoid name clashes). Perhaps I am missing something
but this seems to me foolproof and much cleaner than the alternatives.
George
Luke Plant
> Having said that, I'm in favour of the first approach mentioned in the
> wiki (store state on request) and I'm surprised it doesn't seem to
> have any traction in this thread. The only argument against is "it's
> unusual to have a class where you can't store state on self" but even
> that can be overcome with an appropriate __setattr__/__getattr__ that
> access self.request.state under the hood. Unless someone tries
> self.__dict__[attr], it will look as if the state is stored on the
> view although it's actually stored in the request (or better
> request.state to avoid name clashes). Perhaps I am missing something
> but this seems to me foolproof and much cleaner than the alternatives.
This breaks if you set any values in __init__ - since the request object
doesn't exist to store state on.
Vinay Sajip
On Oct 2, 1:01 pm, Carl Meyer <carl.j.me...@gmail.com> wrote:
> Again, arguments to __init__ are not the issue. What would have to be
> checked is any assignment to self that takes place in __init__. How do
> you propose to check that?
>
instance from just randomly assigning to self.X, unless X were named
in __slots__.
Likewise, __setattr__ can be used to check for assignments to self.
Of course, both might be capable of being subverted, but could be
helpful in catching inadvertent mistakes.
Regards,
Vinay Sajip
George Sakkis
> On Sat, 2010-10-02 at 09:20 -0700, George Sakkis wrote:
> > Having said that, I'm in favour of the first approach mentioned in the
> > wiki (store state on request) and I'm surprised it doesn't seem to
> > have any traction in this thread. The only argument against is "it's
> > unusual to have a class where you can't store state on self" but even
> > that can be overcome with an appropriate __setattr__/__getattr__ that
> > access self.request.state under the hood. Unless someone tries
> > self.__dict__[attr], it will look as if the state is stored on the
> > view although it's actually stored in the request (or better
> > request.state to avoid name clashes). Perhaps I am missing something
> > but this seems to me foolproof and much cleaner than the alternatives.
>
> This breaks if you set any values in __init__ - since the request object
> doesn't exist to store state on.
unusual to have a class where you can't store state on self while in
the __init__". The __call__ and copy proposal has this limitation too,
while being more error prone and less efficient overall.
George
Łukasz Rekucki
> 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.
This largely depends on what you mean by "initialization". There are
actually two levels of this: 1) per-request initialization based on
the request 2) per-view initialization on application startup.
The __init__ method in current implementation[1] falls in to the
second category (independently of the ability to pass in any arguments
as noted by Carl Meyer). This kind of settings should never mutate for
the reasons given by Vince Veselosky. Adding a rule: "don't mutate
objects assigned to self in __init__" doesn't really feel like a
normal use of a Python class.
I agree with you that, subclassing is the prefered way of customizing
views. The fact that class attributes are shared by instances should
be pretty clear to any Python programmer with OOP knowledge (same as
the fact that default arguments for functions are shared between
invocations).
To sum this up, I think the important questions are:
1) Do View instances need to share anything ? I say: yes.
2) What is the purpose of this shared objects ?
I say: customization of *default* view settings (like default
redirect path, default template, default page size in pagination).
3) How do we make this shared settings immutable between requests
(this has nothing to do with thread-safety) ?
This is largely the topic of this thread. Deep copying it
everytime is surely the most secure option, but not very efficient.
Another way is to keep this settings as class attributes and assume
the user is sane enough not to share mutable object between class
instances.
>
> 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.
I don't consider this weird. The view's contract is: "give me a
request, I'll give you a response". In OOP terms, it's a just a
Factory of HttpResponses.
>
>> 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.
Same common knowledge exists concerning class attributes. Lets reuse that.
>>
>> 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.
I don't share your feeling about this. Django already uses a similar
idiom in ModelForms - you pass in a base form, some extra options and
get a new subclass. Also, I would consider this as a shortcut for
advanced users and advice beginners to use subclassing.
[1]: http://github.com/bfirsh/django-class-based-views
--
Łukasz Rekucki
Patryk Zawadzki
<rus...@keith-magee.com> wrote:
> On Sat, Oct 2, 2010 at 7:05 PM, Patryk Zawadzki <pat...@pld-linux.org> wrote:
>> 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.
> Ok, now show me keyword in Python that identifies a class as a
> "utility class". Or the page in the Python documentation where it
> talks about "utility classes" and how you identify them and use them
> properly.
There is no Python documentation that teaches you how to write good
and robust code. There are only docs that tell you what "print" or
"raise" does. I wouldn't pick software engineering during my studies
if I expectes language references to teach me all there was about
programming.
> You can't, because Python doesn't make that distinction. Python has
> classes. Class instances have state. Period. People certainly write
> classes that are stateless. But providing a base API based around
> classes, encouraging people to subclass and add their own behavior,
> and expecting *documentation* to prevent them from using self is
> complete folly IMHO.
I was going to respond to that. I wanted to mention things like
compiled languages and their expectance of you being able to write
good code. I respect you too much to drag you through that. After all
I am not forced to use whatever the standard becomes. If I am ever
forced to use this sort of class-based views, I'll file bugs, until
then I'll try to remain quiet. There are more important decisions to
be made.
Cheers,
--
Patryk Zawadzki
Russell Keith-Magee
Agreed. The "just document" approach is a non-starter; the 'error when
you set attribute on self' is at least a technically viable option.
My problem with the error-based approach is entirely conceptual -- it
feels to me like throwing the baby out with the bathwater. I'm having
a lot of trouble with the idea that class instances have state in
every other Python class instance, and if we introduce an API that
physically prohibits instance state that -- no matter how good the
technical reasoning -- we're going to:
* Cause confusion amongst new users as to why this seemingly obvious
thing can't be done
* Ignore the legitimate occasions where using state is a useful
architectural approach.
>> 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
>
> Again, arguments to __init__ are not the issue. What would have to be
> checked is any assignment to self that takes place in __init__. How do
> you propose to check that?
I don't. You've got me -- in my haste to propose an idea, I hadn't
considered assigning state in the constructor.
I'm thinking on my feet at the moment, but the only way I can think to
resolve this would be to remove the copy() call; effectively making
the __call__() a factory that produces a new class instance that
shares all the methods of the class-based view, but none of the state
created at initialization. However, this is starting to get into the
same territory as why the __new__ approach isn't particularly
appealing.
Yours,
Russ Magee %-)
Russell Keith-Magee
> On Oct 1, 7:26 am, Russell Keith-Magee <russ...@keith-magee.com>
> wrote:
>>
>> I've just added a summary of the last thread on class-based views [1].
>
> Thanks for writing this up. Having missed the discussion on that
> thread, the summary saved me a whole lot of 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.
>
>> Lets be clear here - I don't particularly like *any* of the options on
>> the table so far.
>
> Before getting on topic, I'd like to note that these two paragraphs
> and the general lack of consensus make me a bit uneasy. I'd rather see
> the feature postponed to 1.4 or the October 18 date moved later than
> ending up with a poor solution. With the backwards compatibility
> policy being what it is, design mistakes become very costly.
In fairness, my position of "consensus" was based on the many previous
mailing list discussions that have occurred, as well as the outcome of
a couple of in-person discussions during sprints of the major players.
It's clear from this revival of the discussion that this consensus
wasn't as strong as I thought it was.
I completely agree that we don't want to rush this. The upside is that
if we *can* reach consensus, it isn't going to require a whole lot of
code changes; We're arguing about how to architect the base class, but
once we've got that sorted out, Ben's patch is feature complete and
shouldn't be too hard to adapt to any base class along the lines of
what we've been describing.
> Having said that, I'm in favour of the first approach mentioned in the
> wiki (store state on request) and I'm surprised it doesn't seem to
> have any traction in this thread. The only argument against is "it's
> unusual to have a class where you can't store state on self" but even
> that can be overcome with an appropriate __setattr__/__getattr__ that
> access self.request.state under the hood. Unless someone tries
> self.__dict__[attr], it will look as if the state is stored on the
> view although it's actually stored in the request (or better
> request.state to avoid name clashes). Perhaps I am missing something
> but this seems to me foolproof and much cleaner than the alternatives.
I can see two problems here:
* You still need to assign instance variables during construction; if
you call self.foo during __init__(), you don't have a request; so this
means we need to dynamically swap in a request-based
__setattr__/__getattr__ as part of __call__.
* The more complicated things are, the more likely they are to break
in interesting and creative ways. Futzing with __getattr__/__setattr__
(especially if it's happening dynamically during class use) strikes
me as the sort of thing that has the potential to backfire in really
unusual ways, especially if people get comfortable with the idea that
this class-based view really is just a class, and they start pushing
the envelope.
Yours,
Russ Magee %-)
Russell Keith-Magee
> On 2 October 2010 12:32, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>> 2010/10/2 Łukasz Rekucki <lrek...@gmail.com>:
>>> On 2 October 2010 10:34, Russell Keith-Magee <rus...@keith-magee.com> wrote:
> To sum this up, I think the important questions are:
>
> 1) Do View instances need to share anything ? I say: yes.
Agreed.
> 2) What is the purpose of this shared objects ?
> I say: customization of *default* view settings (like default
> redirect path, default template, default page size in pagination).
Shared settings from the constructor -- agreed. Instance variables
that form part of view logic are another (albiet related) issue.
> 3) How do we make this shared settings immutable between requests
> (this has nothing to do with thread-safety) ?
It's entirely about thread safety; if you've got code modifying any
instance variable, and those instance variables don't have isolated
state between class instances, thread safety will be the first
manifestation.
> This is largely the topic of this thread. Deep copying it
> everytime is surely the most secure option, but not very efficient.
There are also some objects that aren't deep-copyable.
> Another way is to keep this settings as class attributes and assume
> the user is sane enough not to share mutable object between class
> instances.
... and that's a *really* big assumption, and one that has potential
to bite users on the butt really easily -- especially new developers.
>>> 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.
>
> I don't share your feeling about this. Django already uses a similar
> idiom in ModelForms - you pass in a base form, some extra options and
> get a new subclass. Also, I would consider this as a shortcut for
> advanced users and advice beginners to use subclassing.
I'm not sure I see the comparison. ModelForms are declared using a
metaclass; instances are defined in the normal way that every class is
defined. There is a modelform_factory() et al, but they're not class
methods; they're standalone functions that can be used to construct
new classes.
MyView.with(xxx) -- a class method that is a class factory -- isn't a
construct with precedent in Django's codebase AFAICT.
Yours
Russ Magee %-)
David P. Novakovic
taking everything in and not really replying to anyone specifically.
I _really_ like the idea of View being synonymous with a ResponseFactory.
Using __call__:
The view base class can take *args and **kwargs and apply them lazily
when __call__ is called. This is basically like the copy+call one,
except there is no copy,
class ResponseFactory: #might as well be called View
def __init__(self,*args,**kwargs): # discourage args/kwargs
self.args = args
self.kwargs = kwargs
def __call__(self,*args,**kwargs):
response = self.__class__(*self.args,
**self.kwargs).dispatch(*args, **kwargs)
return response
No copy. An explicit contract, with an understood pattern (Factory)
that the initial args and kwargs will be applied to every instance
created from this one (and thus should be considered volatile).
Even though its trivially different, it feels a lot cleaner in my head :)
D
> --
> 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.
>
>
Ian Lewis
<rus...@keith-magee.com> wrote:
>> The issue is not only the frequency of failure, but how explicit/clear
>> it is. The failure here is so obscure and difficult to track down, it
>> is likely to generate an outsize support burden. In contrast, raising
>> an error on assigning to self can be completely explicit; no hours of
>> painful and confusing debugging necessary.
>>
>> Let's also be clear to distinguish "just document that you don't store
>> state on self" vs "raise an error if you set an attribute on self."
>> These are proposals with very different characteristics in terms of
>> likely confusion and support burden.
>
> Agreed. The "just document" approach is a non-starter; the 'error when
> you set attribute on self' is at least a technically viable option.
>
> My problem with the error-based approach is entirely conceptual -- it
> feels to me like throwing the baby out with the bathwater. I'm having
> a lot of trouble with the idea that class instances have state in
> every other Python class instance, and if we introduce an API that
> physically prohibits instance state that -- no matter how good the
> technical reasoning -- we're going to:
>
> * Cause confusion amongst new users as to why this seemingly obvious
> thing can't be done
>
> * Ignore the legitimate occasions where using state is a useful
> architectural approach.
While I'm in the "one singleton view instance is best" camp and think
that storing some state on the request and some on the view is a bit
gross, I understand Russell's arguments. New users are simply going to
save stuff on self no matter how much we complain, document etc. It's
simply a reality that likely can't be helped much.
Other frameworks seem have View/Handler instances per request, such as
appengine's webapp so there is some precedent for creating an instance
per request.
http://code.google.com/appengine/docs/python/gettingstarted/handlingforms.html
Flask seems to do it the callable singleton way:
http://flask.pocoo.org/docs/patterns/lazyloading/
Is there any precedent in other frameworks for doing it the singleton
way? Talking a bit about how other frameworks/libraries do this might
be a good idea to figure out what Joe User is likely to do.
--
Ian
Łukasz Rekucki
> 2010/10/3 Łukasz Rekucki <lrek...@gmail.com>:
>> On 2 October 2010 12:32, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>>> 2010/10/2 Łukasz Rekucki <lrek...@gmail.com>:
>>>> On 2 October 2010 10:34, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>
>> 3) How do we make this shared settings immutable between requests
>> (this has nothing to do with thread-safety) ?
>
> It's entirely about thread safety; if you've got code modifying any
> instance variable, and those instance variables don't have isolated
> state between class instances, thread safety will be the first
> manifestation.
What I meant is that you don't need multi-threaded environment to make
the problem occur - just 2 sequential requests.
>
>> Another way is to keep this settings as class attributes and assume
>> the user is sane enough not to share mutable object between class
>> instances.
>
> ... and that's a *really* big assumption, and one that has potential
> to bite users on the butt really easily -- especially new developers.
IMHO, it falls into the same class of assumptions as "people won't use
mutable objects as default arguments to functions". It's true that
it's a common mistake among developers new to OOP in Python. But it's
a Python-specific problem, not Django-specific.
>>>> 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.
>>
>> I don't share your feeling about this. Django already uses a similar
>> idiom in ModelForms - you pass in a base form, some extra options and
>> get a new subclass. Also, I would consider this as a shortcut for
>> advanced users and advice beginners to use subclassing.
>
> I'm not sure I see the comparison. ModelForms are declared using a
> metaclass; instances are defined in the normal way that every class is
> defined. There is a modelform_factory() et al, but they're not class
> methods; they're standalone functions that can be used to construct
> new classes.
>
> MyView.with(xxx) -- a class method that is a class factory -- isn't a
> construct with precedent in Django's codebase AFAICT.
We can leave this as a standalone function if that changes anything.
Another common pattern in use is an instance method that is a instance
factory. Going up one level seems natural.
PS. I'll try to put up some code today, to represent this approach. Is
it okay to add this to the wiki page ?
--
Łukasz Rekucki
Łukasz Rekucki
> Flask seems to do it the callable singleton way:
>
> http://flask.pocoo.org/docs/patterns/lazyloading/
>
singleton view instance here - just a proxy, that imports a function
on first use. Although functions are singleton by their nature.
> Is there any precedent in other frameworks for doing it the singleton
> way? Talking a bit about how other frameworks/libraries do this might
> be a good idea to figure out what Joe User is likely to do.
+1 on having a compare table between other frameworks.
--
Łukasz Rekucki
Waldemar Kornewald
first of all, I'd like to say that I fully agree that the final
solution has to be thread-safe and has to support storing state on
self.
On Oct 2, 12:32 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
> 2010/10/2 Łukasz Rekucki <lreku...@gmail.com>:
> > __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.
is to have them in urlpatterns, right? You can still do that:
(r'^path$', 'MyView', {'option1': value1, 'option2': ...}),
The other use-case is to pass initialization arguments in your
views.py, but that can easily be replaced with subclassing:
class MyView(View):
option1 = value1
...
Indeed, this doesn't completely solve the problem of shared state
because value1 etc. will be shared, but neither does any of the other
solutions. However, with __new__ any *internal* state you store during
__init__ (e.g., self.state = [1, 2]) will be thread-safe (you can
later safely do self.state.append(3)) and that's IMHO an important
advantage over the __init__+copy() approach.
Independent of the approach, passing mutable arguments to __init__
isn't such a huge issue if you think of those mutable arguments as
*globally* constructed. Global mutable objects just aren't thread-safe
and that's one of the first things you have to learn when working with
multi-threaded code. So, even the __init__+copy() approach IMHO
doesn't make this problem really dramatic. After all, you're
constructing a *global* object. The default assumption is that it
"obviously" can't be thread-safe. To me, it's actually the other way
around: How the heck would I even know just by looking at a views.py
(i.e. without reading the documentation) that the *global* view
**instance** (!) is in fact thread-safe because it copies itself?
> 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.
this is not much more magical than having non-obvious thread-safety
due to copy(). None of the solutions are perfect, but IMHO the thread-
safety advantages of the __new__ approach (i.e., internal state
created in __init__ is thread-safe) outweigh this minor detail because
bugs due to thread-safety issues are harder to track down.
Bye,
Waldemar Kornewald
Russell Keith-Magee
> On 3 October 2010 04:44, Russell Keith-Magee <rus...@keith-magee.com> wrote:
>> 2010/10/3 Łukasz Rekucki <lrek...@gmail.com>:
>>>>> 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.
>>>
>>> I don't share your feeling about this. Django already uses a similar
>>> idiom in ModelForms - you pass in a base form, some extra options and
>>> get a new subclass. Also, I would consider this as a shortcut for
>>> advanced users and advice beginners to use subclassing.
>>
>> I'm not sure I see the comparison. ModelForms are declared using a
>> metaclass; instances are defined in the normal way that every class is
>> defined. There is a modelform_factory() et al, but they're not class
>> methods; they're standalone functions that can be used to construct
>> new classes.
>>
>> MyView.with(xxx) -- a class method that is a class factory -- isn't a
>> construct with precedent in Django's codebase AFAICT.
>
> We can leave this as a standalone function if that changes anything.
> Another common pattern in use is an instance method that is a instance
> factory. Going up one level seems natural.
>
> PS. I'll try to put up some code today, to represent this approach. Is
> it okay to add this to the wiki page ?
Feel free. Even if we don't end up adopting your proposal, it's
helpful for future generations to know that it *was* proposed (and if
it wasn't accepted, why it wasn't accepted).
Yours,
Russ Magee %-)
Russell Keith-Magee
> On Sun, Oct 3, 2010 at 11:20 AM, Russell Keith-Magee
> <rus...@keith-magee.com> wrote:
> While I'm in the "one singleton view instance is best" camp and think
> that storing some state on the request and some on the view is a bit
> gross, I understand Russell's arguments. New users are simply going to
> save stuff on self no matter how much we complain, document etc. It's
> simply a reality that likely can't be helped much.
>
> Other frameworks seem have View/Handler instances per request, such as
> appengine's webapp so there is some precedent for creating an instance
> per request.
>
> http://code.google.com/appengine/docs/python/gettingstarted/handlingforms.html
I don't think you'll find any argument that having an instance per
request would solve all the problems that this thread has described.
The issue is how to declare a view that is able to be instantiated on
a instance-per-request basis while simultaneously allowing decorators
to be easily wrapped around that view.
Yours,
Russ Magee %-)
George Sakkis
> Other frameworks seem have View/Handler instances per request, such as
> appengine's webapp so there is some precedent for creating an instance
> per request.
>
> Flask seems to do it the callable singleton way:
>
> http://flask.pocoo.org/docs/patterns/lazyloading/
>
> Is there any precedent in other frameworks for doing it the singleton
> way? Talking a bit about how other frameworks/libraries do this might
> be a good idea to figure out what Joe User is likely to do.
"These requests result in a new instance of the WSGIController being
created, which is then called with the dict options from the Routes
match." (http://pylonshq.com/docs/en/1.0/controllers/). No mentioning
about setting state on self and no __init__ on controllers (even the
base WSGIController doesn't define __init__).
George
George Sakkis
wrote:
> >> 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
>
> > Again, arguments to __init__ are not the issue. What would have to be
> > checked is any assignment to self that takes place in __init__. How do
> > you propose to check that?
>
> I don't. You've got me -- in my haste to propose an idea, I hadn't
> considered assigning state in the constructor.
setattr(self.request.state, attr, value) you can check that; at
__init__ there is no self.request yet.
George
Ian Lewis
> On 3 October 2010 06:12, Ian Lewis <ianm...@gmail.com> wrote:
>> Flask seems to do it the callable singleton way:
>>
>> http://flask.pocoo.org/docs/patterns/lazyloading/
>>
> I never used Flask, so I might be missing something, but I don't see a
> singleton view instance here - just a proxy, that imports a function
> on first use. Although functions are singleton by their nature.
I'm not a flask expert either but under "Loading Late" there is a
LazyView which I believe is just a callable object.
And later under that he adds a LazyView instance to a url rule so I think it's
simply a "view is a callable" pattern.
--
Ian
Ian Lewis
<rus...@keith-magee.com> wrote:
>> Other frameworks seem have View/Handler instances per request, such as
>> appengine's webapp so there is some precedent for creating an instance
>> per request.
>>
>> http://code.google.com/appengine/docs/python/gettingstarted/handlingforms.html
>
> I don't think you'll find any argument that having an instance per
> request would solve all the problems that this thread has described.
> The issue is how to declare a view that is able to be instantiated on
> a instance-per-request basis while simultaneously allowing decorators
> to be easily wrapped around that view.
I was sort of referring to the __new__() method you are describing to
allow state to be stored on self.
A class instance that can be wrapped by a decorator to me is simply a
callable instance and people are just
going to have to get used to the idea that self can't be modified
safely. Anything else is a bad hack that will likely
cause even more headaches down the road.
That said, even Django itself has code that falls victim to storing
state on self in a callable class based view (See:
django.contrib.formtools.preview.FormPreview). So I can imagine many
people will be confused, but it is what it is.
--
Ian
André Eriksson
On Oct 3, 1:02 pm, Russell Keith-Magee <russ...@keith-magee.com>
wrote:
> > On Sun, Oct 3, 2010 at 11:20 AM, Russell Keith-Magee
> > <russ...@keith-magee.com> wrote:
> > While I'm in the "one singleton view instance is best" camp and think
> > that storing some state on the request and some on the view is a bit
> > gross, I understand Russell's arguments. New users are simply going to
> > save stuff on self no matter how much we complain, document etc. It's
> > simply a reality that likely can't be helped much.
>
> > Other frameworks seem have View/Handler instances per request, such as
> > appengine's webapp so there is some precedent for creating an instance
> > per request.
>
> I don't think you'll find any argument that having an instance per
> request would solve all the problems that this thread has described.
> The issue is how to declare a view that is able to be instantiated on
> a instance-per-request basis while simultaneously allowing decorators
> to be easily wrapped around that view.
>
> Yours,
> Russ Magee %-)
discussion, but the solution I proposed handles decorators just fine.
Furthermore, you can extend it to allow for decorators by
subclassing.
Initial code:
class BaseView(object):
def __new__(cls, *args, **kwargs):
obj = super(BaseView, cls).__new__(cls)
# Call __init__ manually since it will not be called
# after this method (since __new__ returns a HttpResponse
# and not a BaseView subclass).
obj.__init__(*args, **kwargs)
return obj.view() # Or other name
class MyView(BaseView):
def __init__(self, request):
self.request = request
def view(self):
return HttpResponse(self.request.path)
login_required(MyView)))
This will create a view that requires login. We can extend this by
introducing a helper function class_decorator that takes a regular
decorator and turns it into a view class:
def class_decorator(decorator):
class Cls(BaseView):
def __new__(cls, *args, **kwargs):
def view(*args, **kwargs):
return super(Cls, cls).__new__(cls, *args, **kwargs)
return decorator(view)(*args, **kwargs)
return Cls
We can then create a LoginRequiredView for instance:
LoginRequiredView = class_decorator(login_required)
class MyView2(LoginRequiredView):
self.request = request
def view(self):
return HttpResponse(self.request.path)
decorators by multiple inheritance.
AUsersView = class_decorator(user_passes_test(lambda u: "a" in
u.username.lower()))
class MyView3(LoginRequiredView, AUsersView):
"""
View that requires users to be logged in and
have a username that contain an 'a'.
def __init__(self, request):
self.request = request
def view(self):
return HttpResponse(self.request.path)
André Eriksson
André Eriksson
decorated views can be subclassed. All other functionality is retained.
Andrew Godwin
> * Ignore the legitimate occasions where using state is a useful
> architectural approach.
>
I'd just like to add more noise to the signal and reiterate this -
storing state on self (or request) leads to much cleaner, less fragile,
more subclassable views (in my own experience, having written both a set
of generic views with and without state like this).
I personally think self is the place to store this state (since, as Russ
keeps saying, that's how nearly all Python classes do it), but storing
it on request isn't too bad - it just "feels" more wrong to me, but
then, middleware does it already.
However, subjectivity isn't going to win through here. Either solution
is fine, just don't force me to pass everything around as arguments
between functions. That's not looking likely, but some people here seen
to want to have that kind of purity at the expense of usability -
dammit, Jim, I'm an engineer, not a mathematician.
Andrew
Warren Smith
I'd just like to add more noise to the signal and reiterate this - storing state on self (or request) leads to much cleaner, less fragile, more subclassable views (in my own experience, having written both a set of generic views with and without state like this).
I personally think self is the place to store this state (since, as Russ keeps saying, that's how nearly all Python classes do it), but storing it on request isn't too bad - it just "feels" more wrong to me, but then, middleware does it already.
Seriously though, the key considerations and points made in this thread need to be extracted and put somewhere in the django docs, preferably in a place that doesn't distract a brand new user but also gets read soon after they complete the tutorial and begin digging a little deeper. Knowing the design considerations that went into whatever path is taken is, IMHO, invaluable.
I am willing to do the work to make that happen if nobody else is interested. I have not yet contributed to the corpus of django documentation, but perhaps this an opportunity to do so.
However, subjectivity isn't going to win through here. Either solution is fine, just don't force me to pass everything around as arguments between functions. That's not looking likely, but some people here seen to want to have that kind of purity at the expense of usability - dammit, Jim, I'm an engineer, not a mathematician.
--
Warren Smith
legutierr
wrote:
> I completely agree that we don't want to rush this. The upside is that
> if we *can* reach consensus, it isn't going to require a whole lot of
> code changes; We're arguing about how to architect the base class, but
> once we've got that sorted out, Ben's patch is feature complete and
> shouldn't be too hard to adapt to any base class along the lines of
> what we've been describing.
"how to architect the base class", I think that there is an
opportunity to discuss other elements of the implementation that have
been neglected up to now.
While bfirsh's implementation may be *almost* feature complete, it
seems to be lacking one major feature that will be important to many,
if not most, users; specifically, the ability to easily return data
*other than* html rendered from a template. A large number of users,
if not a majority, will want to be able to use class-based views to
return JSON data. Others will want to render and return XML or PDF
data. Some users may even want to use django-annoying's @render_to
and @ajax_request decorators, and would have classy views return
dictionaries instead of HTTPResponse objects.
Of course, users could subclass the base View class to do any of these
things, but then they would be discarding most of this module's useful
functionality. The real power of class-based generic views will be
found in the subclasses included in the module (ListView, DetailView,
etc.). The real power will be the ability to re-use the generic list-
processing, detail-processing, and form-processing code, not in the
high-level architecture of the base class.
Users, I think, should be shown an obvious and intuitive path by which
they may reuse the data-processing logic of these generic view objects
without being limited to a single output format, and without having to
jump through hoops in terms of subclassing disparate objects and
overriding tricky methods that differ from subclass to subclass in
terms of both signature and implementation.
At this moment in time, TemplateView is a base class of all of View's
descendant classes in bfirsh's implementation. All inheritance flows
through TemplateView for the entire library. Furthermore, every data-
processing subclass of View references TemplateView's
render_to_response method, which is oriented in its implementation and
(more importantly) its signature only towards template processing.
As a result, any user wanting to implement a JsonView, an XMLView, or
a PDFView will need to subclass a subclass of TemplateView in order to
reuse the generic views' data-processing code, and will also have to
know quite a bit about the intricacies of how the different methods
interact inside each of these classes. This sub-optimal for a number
of reasons:
1. Views that neither render html nor use templates will report that
they are TemplateView instances (via isinstance()).
2. In order to create a suite of generic JsonViews, for example,
many different classes will have to be subclassed, and the methods
that are overridden will not be consistent across those subclasses.
3. It is unlikely that any of this will be documented, and the
overriding rules will not be intuitive nor obvious, leading to
confusion and support requests.
A moderate refactoring of the internals of the implementation, along
with a willingness to see some of the lower-level implementation
details of the generic view objects as part of the public API, could
resolve these issues. My own suggestion is as follows:
* First, treat data processing and retrieval as separable from
rendering. Create a bright line of separation between the two
conceptual elements of the view (data and rendering), and do it early
on, at a high level, inside dispatch(). Instead of expecting the
ListView.GET to return an HTTPResponse, for example, have it return a
context or a context dictionary that could be reused with several
render_result() implementations.
* Second, have the dispatch method in View call
render_result(context_dict), which will raise a NotImplemented
exception in the base class, but will return in the subclass whatever
the return data might be. This will be the locus of control for
different data implementations, and can largely be implemented without
any knowledge of the data processing details.
* Third, provide different implementations of render_result()
through the use of different mixins, each targeting a different output
style (template, json, XML, PDF, etc.). That way, the logic that
handles data processing and retrieval can be re-used regardless of
what the data output may be, and vice-versa.
* Finally, handle redirects, reloads, or 404's through exceptions,
which would be processed inside dispatch() as well. Using exceptions
for normal flow of control is generally frowned upon, but here it
would allow for a simplified API description for calls to GET(),
POST(), etc.: These methods would return a dictionary in the standard
case, and raise specialized exceptions for redirects, reloads, or file-
not-founds and other deviations from the "norm."
The current implementation is already using mixins in order to
maximize code reuse in form processing. It seems that using mixins to
modify rendering behavior would also be appropriate. And if mixins
are unacceptable for the documented API ("mixins are a relatively
unused part of Python, and multiple inheritance can cause some odd
bugs"), the same compartmentalization can be achieved, albeit in a
more repetitive way (copy and paste), by simply overriding
render_result(context_dict) in direct subclasses of ListView,
DetailView, etc. Most significantly, however, it would be much easier
to document how a user can add his or her own response data type if
such a thing were limited to nothing more than overriding
render_result(context_dict).
Of course this is just one example of an approach. There are other
approaches that would also be an improvement over what is implemented.
If I have a more over-arching point here it is this: the API for the
base class may be the most important issue to decide before committing
to include this feature in version 1.3, but it is certainly not the
only one. Rather than assuming that all other issues are settled, I
think it might be more correct to see the architecture discussion as
having preempted the other conversations that need to take place. For
what it's worth, I would personally love to see this feature in
version 1.3, but it seems more important that it be done right, and I
am sure that everyone participating in this discussion would agree.
The lower-level aspects of the implementation need to be more
thoroughly vetted before this feature should be considered suitable
for inclusion, IMHO.
Regards,
Eduardo Gutierrez
Andrew Godwin
> * First, treat data processing and retrieval as separable from
> rendering. Create a bright line of separation between the two
> conceptual elements of the view (data and rendering), and do it early
> on, at a high level, inside dispatch(). Instead of expecting the
> ListView.GET to return an HTTPResponse, for example, have it return a
> context or a context dictionary that could be reused with several
> render_result() implementations.
>
This is problematic if I want to return something that isn't a context,
or like it (for example, if I'm rendering images, or fetching content
wholesale out of a database).
So, bfirsh's previous iteration had content formats built into the base
class - I ripped it out and replaced it with a simpler base class, and
he seemed to agree, so that's where we currently are.
My main concern is getting rid of the simplicity - e.g. of calling
render() on a template/context mix. In this aforementioned previous
iteration, if I wanted to supply custom context to a custom template, I
had to override both self.get_template_name() and self.get_context() -
even though it would have been a lot easier to override GET and just
call self.render(). It's a similar problem to passing everything around
as keyword arguments - reusability doesn't turn out to be much better
than starting from scratch.
(FWIW, I've written several hundred LOC with both styles and it was a
lot less work when there was less abstraction)
I just don't want us to build in all these abstraction layers and have
the ability to customise everything perfectly but in a verbose fashion.
That said, if render_result() in your example just calls a normal
render() method itself, it's easy to not use it if you don't have to, so
a reasonable approach to get both of these angles in is possible.
Also, what use cases do people have for returning arbitary pages as JSON
or XML?
I'm not saying there aren't any - far from it - but I much prefer
evidence to speculation, so some concrete examples would be great
Finally, we have to be wary of making this automatic on all views, or
something similar - it could potentially be a
security/warm-fuzzy-feeling risk, as some people put things into the
context that aren't necessarily for display to the user (though you'd
have to be pretty stupid to put anything actually sensitive in there).
(There's also the issue of whether you apply context processors to the
JSON replies, and so forth, but that's going even further down the
rabbit hole)
Andrew
Alex Gaynor
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To post to this group, send email to django-d...@googlegroups.com.
> To unsubscribe from this group, send email to
> django-develop...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>
Given that the primary complain against the __new__ solution is that
it's unintuitive to Python programmers that instantiating their class
results in some other class, why not simply go with a more explicit
classmethod. Simply used as:
url("^articles/$", ArticlesView.dispatch).
It seems to address all concerns: it's explicit, safe to assign
attributes, Pythonic, and fails loudly if used improperly (using
AritlcesView would result in an Exception being throw since it doesn't
return an HttpResponse, and using, and ArticlesView() an error about
not being callable).
Last idea, I swear,
Alex
--
"I disapprove of what you say, but I will defend to the death your
right to say it." -- Voltaire
"The people's good is the highest law." -- Cicero
"Code can always be simpler than you think, but never as simple as you
want" -- Me
Jacob Kaplan-Moss
> Given that the primary complain against the __new__ solution is that
> it's unintuitive to Python programmers that instantiating their class
> results in some other class, why not simply go with a more explicit
> classmethod. Simply used as:
>
> url("^articles/$", ArticlesView.dispatch).
Interesting. So `View.dispatch` would look something like:
@classmethod
def dispatch(cls, request, *args, **kwargs):
instance = cls()
callback = getattr(instance, request.method.lower())
return callback(request, *args, **kwargs)
(e.g. create an instance of `cls` then call `get`/`post`/etc. on it).
Other than the slight cruftyness of `dispatch`, I quite like it. Seems
it addresses most of the issues we've found so far. Doesn't solve the
"pass configuration into __init__" idea, but frankly I didn't think
much of that idea in the first place.
To my eyes this is the best solution raised so far.
Jacob
Luke Plant
> Given that the primary complain against the __new__ solution is that
> it's unintuitive to Python programmers that instantiating their class
> results in some other class, why not simply go with a more explicit
> classmethod. Simply used as:
>
> url("^articles/$", ArticlesView.dispatch).
>
> It seems to address all concerns: it's explicit, safe to assign
> attributes, Pythonic, and fails loudly if used improperly (using
> AritlcesView would result in an Exception being throw since it doesn't
> return an HttpResponse, and using, and ArticlesView() an error about
> not being callable).
>
> Last idea, I swear,
> Alex
>
I've put together a repos with a bunch of scripts that demonstrate in
about a page of code the essence of what we are trying to achieve
(without all the detail about the actual dispatch to HTTP verbs etc).
Start with naive.py:
http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/naive.py
(Just run as 'python naive.py' to see the output). Browse up a level to
see the other variants.
I implemented Alex's latest idea in classmethod.py (with 'as_view'
instead of 'dispatch', to avoid name clashes). It suffers from the same
limitation as the __new__ in that you can't pass configuration data to
the init. But you can do a 'configure' class method, as you can also do
with __new__
Personally, I now prefer these: first place:
http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/classmethod.py
Close second:
http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/magicnew_with_configure.py
Given the limitations, I'm no longer that bothered about the fact that
the __new__ method returns something completely unexpected, but Alex's
classmethod solution seems like a nice improvement on it, though it
makes a URLconf very slightly more verbose.
Luke
--
"Doubt: In the battle between you and the world, bet on the world."
(despair.com)
Luke Plant || http://lukeplant.me.uk/
Łukasz Rekucki
The only problem is decorators: You can't just simply apply
login_required() to the class or the dispatch() method. Otherwise, I
like this approach.
--
Łukasz Rekucki
Luke Plant
> The only problem is decorators: You can't just simply apply
> login_required() to the class or the dispatch() method. Otherwise, I
> like this approach.
It works fine:
http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/classmethod.py#cl-40
(I have 'as_view' instead of 'dispatch'). My MyView.dispatch method
could have decorators applied inside the actual class (as long as they
are adjusted for being method decorators and not function decorators, in
common with all other solutions).
Luke Plant
> it's unintuitive to Python programmers that instantiating their class
> results in some other class, why not simply go with a more explicit
> classmethod. Simply used as:
>
> url("^articles/$", ArticlesView.dispatch).
>
> It seems to address all concerns: it's explicit, safe to assign
> attributes, Pythonic, and fails loudly if used improperly (using
> AritlcesView would result in an Exception being throw since it doesn't
> return an HttpResponse, and using, and ArticlesView() an error about
> not being callable).
After playing around with this for a bit, it seems to be the best option
on the table. The only slight API problem I've come across is that it
is *possible* to try to pass configuration arguments to __init__:
class MyView(View):
def __init__(self, myparam=0):
...
view = MyView(myparam=1).as_view
instead of using configure:
view = MyView.configure(myparam=1).as_view
However, as soon as you try to use that view, you will find that your
custom parameter is being ignored. That might cause some head
scratching to the newbie, but nothing like the insidious thread-safety
problems we're trying to avoid.
Łukasz Rekucki
> On Mon, 2010-10-04 at 19:37 +0200, Łukasz Rekucki wrote:
>
>> The only problem is decorators: You can't just simply apply
>> login_required() to the class or the dispatch() method. Otherwise, I
>> like this approach.
>
> It works fine:
>
> http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/classmethod.py#cl-40
>
> (I have 'as_view' instead of 'dispatch'). My MyView.dispatch method
> could have decorators applied inside the actual class (as long as they
> are adjusted for being method decorators and not function decorators, in
> common with all other solutions).
You're right, my bad. It's 8pm and i'm still in work - probably
shouldn't read any more code today :). That said, my preferences would
be: magicnew_with_configure, classmethod, magicnew, copyoncall.
--
Łukasz Rekucki
Łukasz Rekucki
At the risk of being silly again, this can be prevented by a decorator:
class classonlymethod(classmethod):
def __get__(self, instance, owner):
if instance is not None:
raise AttributeError("This method is availble only on the
view class.")
return super(classonlymethod, self).__get__(instance, owner)
Of course, subclasses that want to override as_view() would need to
use the same decorator (classmethod will still work, just won't catch
the error).
>
> Luke
>
>
> --
> "Doubt: In the battle between you and the world, bet on the world."
> (despair.com)
>
> Luke Plant || http://lukeplant.me.uk/
>
> --
> 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.
>
>
--
Łukasz Rekucki
Luke Plant
> Last idea, I swear,
I didn't swear, so here is another slight variation :-) You actually
*call* the classmethod in your URLconf, passing any constructor
arguments to it:
url(r'^detail/author/(?P<pk>\d+)/$',
views.AuthorDetail.as_view(some_param=123),
name="author_detail"),
It returns a newly created view function, which in turn calls your
class.
http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/classmethod2.py
This was, in fact, suggested by Jari Pennanen in May :-(
http://groups.google.com/group/django-developers/msg/997ac5d38a96a27b
It's slightly less verbose for passing parameters, as you don't need a
separate 'configure' call.
George Sakkis
> On Mon, Oct 4, 2010 at 12:08 PM, Alex Gaynor <alex.gay...@gmail.com> wrote:
> > Given that the primary complain against the __new__ solution is that
> > it's unintuitive to Python programmers that instantiating their class
> > results in some other class, why not simply go with a more explicit
> > classmethod. Simply used as:
>
> > url("^articles/$", ArticlesView.dispatch).
>
> Interesting. So `View.dispatch` would look something like:
>
> @classmethod
> def dispatch(cls, request, *args, **kwargs):
> instance = cls()
> callback = getattr(instance, request.method.lower())
> return callback(request, *args, **kwargs)
>
> (e.g. create an instance of `cls` then call `get`/`post`/etc. on it).
>
> Other than the slight cruftyness of `dispatch`, I quite like it. Seems
omit it from the urlconf and have the URLresolver do:
if isinstance(view, type) and issubclass(view, View):
view = view.dispatch
> it addresses most of the issues we've found so far. Doesn't solve the
> "pass configuration into __init__" idea, but frankly I didn't think
> much of that idea in the first place.
George
George Sakkis
> On Mon, 2010-10-04 at 13:08 -0400, Alex Gaynor wrote:
> > Last idea, I swear,
>
> I didn't swear, so here is another slight variation :-) You actually
> *call* the classmethod in your URLconf, passing any constructor
> arguments to it:
>
> url(r'^detail/author/(?P<pk>\d+)/$',
> views.AuthorDetail.as_view(some_param=123),
> name="author_detail"),
>
> It returns a newly created view function, which in turn calls your
> class.
>
It seems there is a corollary to D. Wheeler's quote: "all problems in
computer science can be solved by another level of nested
closures" ;-)
> This was, in fact, suggested by Jari Pennanen in May :-(
>
> http://groups.google.com/group/django-developers/msg/997ac5d38a96a27b
>
> It's slightly less verbose for passing parameters, as you don't need a
> separate 'configure' call.
that you don't want to pass parameters: views.AuthorDetail.as_view().
Unless we "cheat" and have the resolver do:
if isinstance(view, type) and issubclass(view, View):
# if hasattr(view, 'as_view'):
view = view.as_view()
in which case it can be declared in the urlconf simply as
views.AuthorDetail.
George
David P. Novakovic
>
> Since dispatch is going to be defined on the base View class, can't we
> omit it from the urlconf and have the URLresolver do:
>
> if isinstance(view, type) and issubclass(view, View):
> view = view.dispatch
Russ mentioned this one can't be decorated.
I personally love the classmethod idea, so that's +1 from me for that.
D
Marco Chomut
out every time, but my only concern is how confusing an implicit
dispatch would be for developers first using the new class-based Views
system. As long as it was made abundantly clear via the docs what
exactly is happening when your route a url to a View class it really
shouldn't be an issue.
My apologies if it has already been brought up, but I'd also like to
propose that a default context_instance be added as a self variable to
the View class, if for nothing else than to avoid having to write
context_instance=RequestContext(request) in nearly every return
statement, or requiring every one of my apps to be dependent on
something like django-annoying for its render_to() decorator.
George Sakkis
wrote:
>
> > Since dispatch is going to be defined on the base View class, can't we
> > omit it from the urlconf and have the URLresolver do:
>
> > if isinstance(view, type) and issubclass(view, View):
> > view = view.dispatch
>
> Russ mentioned this one can't be decorated.
case, you can always decorate the SomeClassView.dispatch:
url("^articles1/$", ArticlesView.dispatch), #
undecorated
url("^articles2/$", login_required(ArticlesView.dispatch)), #
decorated
url("^articles3/$", ArticlesView), # shortcut of the
first
# url("^articles4/$", login_required(ArticlesView)), # this doesn't
work
There is an asymmetry here but that's not to say it can't be
decorated, you just have to decorate the dispatch method instead of
the class view.
George
Russell Keith-Magee
Yes, but that's the sort of asymmetry I'd like to avoid if possible.
It's easy to document that you can wrap a decorator around anything
you can put in a URL pattern. When you start making exceptions, then
you have to explain the exceptions, and then explain to the people who
don't read and/or understand the explanation... and so on.
Yours,
Russ Magee %-)
Russell Keith-Magee
> On Mon, 2010-10-04 at 13:08 -0400, Alex Gaynor wrote:
>
>> Last idea, I swear,
>
> I didn't swear, so here is another slight variation :-) You actually
> *call* the classmethod in your URLconf, passing any constructor
> arguments to it:
>
> url(r'^detail/author/(?P<pk>\d+)/$',
> views.AuthorDetail.as_view(some_param=123),
> name="author_detail"),
>
> It returns a newly created view function, which in turn calls your
> class.
>
> http://bitbucket.org/spookylukey/django-class-views-experiments/src/tip/classmethod2.py
This looks to me like it could be a winner. The fact that
AuthorDetail(foo=bar).as_view(pork=spam) ignores the 'foo' argument is
a minor wart, but a lot less concerning to me that the potential
threading problems in copy+call, or the 'no state on self' options.
Thanks for working up the examples, Luke!
Russ %-)
Russell Keith-Magee
Ok - so to kick the process into the next phase, I've just created a
Django branch in my bitbucket repo [1] to cover introducing this to
trunk.
I've used the implementation in Ben's github repo [2] as a starting
point -- it looks like David Larlet has already made the changes to
implement the classmethod2 approach.
So far, other than some reorganization to get everything into Django's
tree, I've made two changes:
* Added PendingDeprecationWarnings to the old function-based generic views
* Following a late night IRC conversation, I've done some PEP8
bikeshedding, and renamed the request methods GET(), POST() etc to
lower case get(), post().
Some other issues that still need to be resolved:
* Does RequestFactory need to be added to Django's test tools
(Possibly fixing #9002)?
* Does django.views.generic.utils.coerce_put_post() indicate a change
that needs to be made in Django? (Is there an existing ticket for
this?)
* Are there any outstanding tickets on generic views that will be
closed by merging this branch, and do they ask for any features that
aren't fixed by this branch?
* We need to reference and topic docs to describe the new view classes.
* We need to update the tutorial to use the new view classes
Pull requests gratefully accepted for any of these items :-)
[1] http://bitbucket.org/freakboy3742/django
[2] http://github.com/bfirsh/django-class-based-views
Yours
Russ Magee %-)
Jacob Kaplan-Moss
<rus...@keith-magee.com> wrote:
> Ok - so to kick the process into the next phase, I've just created a
> Django branch in my bitbucket repo [1] to cover introducing this to
> trunk.
I gave this a quick review and nothing huge jumped out. Looks good so far.
One point of concern that came up though: looking at the way as_view
introduces a closure, it occurs to me that the docstring of am
as_view'd class view isn't very useful, which'll break introspection
and things like the admindocs tool. And just using functools.wraps (or
the equivalent) is as_view won't exactly work, either: you'll get the
dispatch() docstring instead.
So unless anyone can think of a reason why it'd be a bad idea, I think
as_view needs to monkeypatch the class's docstring into the returned
closure. Bit of a code smell, but I think it maintains the correct
self-documenting nature of a view, yeah?
> * Does RequestFactory need to be added to Django's test tools
> (Possibly fixing #9002)?
I'm not sure it's directly class-based-views-related, but I believe it
should, yes. It's a useful pattern that makes it into a utils module
in nearly every app I write.
> * Does django.views.generic.utils.coerce_put_post() indicate a change
> that needs to be made in Django? (Is there an existing ticket for
> this?)
Yeah, this has been a wart in Django for a while -- Django doesn't
really "get" PUT very well. Along the same lines,
request.raw_post_data is terribly named. I don't know that there's a
single ticket anywhere, but I'd like to see a general cleanup here.
I don't see this as a blocker for class-based views, though: we have a
narrow feature deadline that I'd like to hit, and then a longer
bug-fix period we can use to clean up PUT/POST and such.
> * Are there any outstanding tickets on generic views that will be
> closed by merging this branch, and do they ask for any features that
> aren't fixed by this branch?
Almost certainly yes :)
We could *really* use a volunteer who can wade through the open
tickets, look at all the GV-related tickets, and answer this question
concretely.
Anyone? Bueller?
Jacob
Andy McKay
> 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?
That's what I've done on quite a few projects and its worked great. As Russell pointed out I lose decoratoring in urls.py, but that's something I don't use anyway.
--
Andy McKay
an...@clearwind.ca
twitter: @andymckay
Luke Plant
> One point of concern that came up though: looking at the way as_view
> introduces a closure, it occurs to me that the docstring of am
> as_view'd class view isn't very useful, which'll break introspection
> and things like the admindocs tool. And just using functools.wraps (or
> the equivalent) is as_view won't exactly work, either: you'll get the
> dispatch() docstring instead.
>
> So unless anyone can think of a reason why it'd be a bad idea, I think
> as_view needs to monkeypatch the class's docstring into the returned
> closure. Bit of a code smell, but I think it maintains the correct
> self-documenting nature of a view, yeah?
Sounds fine to me, and I think it should also copy other function
attributes from the dispatch method, to allow people to apply decorators
like 'csrf_exempt' to the dispatch method and have it work as expected.
We might like to think about whether we add any other mechanism for
decorators. We already have the ability to decorate the result of
'as_view()', which is best suited for things like 'cache_page', and to
decorate methods like dispatch, which is best suited for decorators that
are required for the view to work correctly. A list of decorators could
also be specified as a class/instance attribute and added inside the
'as_view' to the closure that is returned. For the moment, I think
adding this would add too many choices, and if there is a need for it we
can add it as a feature later.
legutierr
> On 04/10/10 17:28, legutierr wrote:
>
> > * First, treat data processing and retrieval as separable from
> > rendering. Create a bright line of separation between the two
> > conceptual elements of the view (data and rendering), and do it early
> > on, at a high level, inside dispatch(). Instead of expecting the
> > ListView.GET to return an HTTPResponse, for example, have it return a
> > context or a context dictionary that could be reused with several
> > render_result() implementations.
>
> This is problematic if I want to return something that isn't a context,
> or like it (for example, if I'm rendering images, or fetching content
> wholesale out of a database).
sufficient for 100% of cases, but that it would be sufficient for 99%
of cases where a standard data dictionary could be used to generate
the response, whatever it may be. And while it is *conceivable* that
one would want to use ListView, DetailView, etc. functionality in
combination with image rendering, or other kind of unexpected content,
it is not likely.
However, it *is* likely that one would want to use ListView,
DetailView, etc. to produce JSON, XML, PDF or other text-like content,
which is the common user expectation that *needs* to be addressed.
Overriding dispatch() to implement less standard functionality, or
overriding get() at the same time as render_result(), would still be
feasible for other cases where a simple data dictionary is
insufficient for rendering. But if it is just a question of how text
is being output, then custom implementation of
render_result(data_dictionary) would be sufficient in 99% of cases.
> So, bfirsh's previous iteration had content formats built into the base
> class - I ripped it out and replaced it with a simpler base class, and
> he seemed to agree, so that's where we currently are.
addressing is the fact that instead of subclassing directly from the
simple base class (which makes no assertion about what type of data is
being returned, a very good thing), ListView, DetailView et al.
subclass TemplateView. I would assert that the actual rendering logic
should be implemented as a mixin, and combined with ListView,
DetailView, etc. in order to produce the user-oriented generic view
classes. That way, alternative rendering implementations would be
much more trivial to add, and without creating a misleading class
hierarchy (i.e., by having the JSONView also be a TemplateView).
> My main concern is getting rid of the simplicity - e.g. of calling
> render() on a template/context mix. In this aforementioned previous
> iteration, if I wanted to supply custom context to a custom template, I
> had to override both self.get_template_name() and self.get_context() -
> even though it would have been a lot easier to override GET and just
> call self.render(). It's a similar problem to passing everything around
> as keyword arguments - reusability doesn't turn out to be much better
> than starting from scratch.
override GET to modify what data is to be put into the context. And
since the TemplateView render_template() implementation would use
self.request to build a RequestContext, you would still be able to use
context processors. The difference is that if you wanted to modify
*both* what data is being added to the context and do custom stuff
with the output data, you would have to override both GET() and
render_result()/render_template(). Or you could just override
dispatch()/as_view() in any case.
> I just don't want us to build in all these abstraction layers and have
> the ability to customise everything perfectly but in a verbose fashion.
> That said, if render_result() in your example just calls a normal
> render() method itself, it's easy to not use it if you don't have to, so
> a reasonable approach to get both of these angles in is possible.
context) in the case of the TemplateViewMixin, just as
render_to_response() does in the current implementation. However, in
the case of a JSONViewMixin, render_result() would process the data
dictionary to produce json output instead. I actually think that this
makes things much less verbose overall.
The trickier question is whether one might want to implement a
MultiViewMixin, capable of outputing *both* html documents and json,
for example by registering MyView.as_view and MyView.as_ajax_view in
urls.py. If you look through the use cases I list below, in each of
these use cases a user would benefit from being able to register two
different views with two different urls using the same view class
implementation. The current implementation makes this impossible
without overriding mostly everything.
> Also, what use cases do people have for returning arbitary pages as JSON
> or XML?
> I'm not saying there aren't any - far from it - but I much prefer
> evidence to speculation, so some concrete examples would be great
* A search result implementation *similar* to Google Instant (say, in
an online store), where the search results are either displayed as a
template rendering, or are returned as JSON for dynamic display
through ajax. The data-processing component here is the same in both
cases; the sole difference is rendering.
* A commenting system that defaults to ajax for submissions, but that
permits users to enter comments via a standard html form if javascript
is turned off on the browser. The same form processing logic could be
reused, but the way that the data is used to create the response is
different.
* A billing system that allows a user to view an invoice as HTML on
the website, but provides a link to output a PDF of the invoice for
hard-copy record-keeping, and re-uses the same get() logic in both
cases, but uses a different rendering engine in each.
* An atom or rss feed that uses the same data in its rendering as the
list of articles that are rendered on the site in html. Urls that are
analogous between the blog's html and feed implementations would
actually point to the same class.
> (There's also the issue of whether you apply context processors to the
> JSON replies, and so forth, but that's going even further down the
> rabbit hole)
render_result() to create a RequestContext there. In fact, it should.
Regards,
Eduardo
David Larlet
Jacob Kaplan-Moss a �crit :
#1282: archive_today generic view should accept a month_format parameter
instead of hardcoding '%b'
Done, through MonthView.month_format
#2367: pagination for date based generic views
Depends on http://github.com/bfirsh/django-class-based-views/pull/2
#8378: Generic views cache-like behavior
Last edited one year ago, not sure if it's still relevant
#13753: Generic views don't redirect to an URL name, like
django.shortcuts.redirect does
Done, no more post_save_redirect argument but View.redirect_to()
#13842: XViewMiddleware fails with class-based views
Must be fixed before class-based views merge.
I hope I didn't forget one, it's really hard to search with Trac...
Best,
David
Łukasz Rekucki
>
>
> Jacob Kaplan-Moss a écrit :
(few more found in this query[1])
#9150: Generic views should accept arguements for form_class (related to #12392)
Mostly fixed by adding "initial" parameter in FormMixin. This could be
changes to get_initial(), so it can be based on request object.
#12669: Return different HttpResponse from direct_to_template
Use a TemplateView with get_response() overriden. The patch on the
ticket adds a response class as a parameter. Should we add a similar
to TemplateView ?
#13953: Generic CRUD views: support for callable in post_ACTION_redirect
There are no post_ACTION_redirect parameters anymore, can be done by
overriding redirect_to()
--
Łukasz Rekucki
legutierr
> > * Does django.views.generic.utils.coerce_put_post() indicate a change
> > that needs to be made in Django? (Is there an existing ticket for
> > this?)
>
> Yeah, this has been a wart in Django for a while -- Django doesn't
> really "get" PUT very well. Along the same lines,
> request.raw_post_data is terribly named. I don't know that there's a
> single ticket anywhere, but I'd like to see a general cleanup here.
>
> I don't see this as a blocker for class-based views, though: we have a
> narrow feature deadline that I'd like to hit, and then a longer
> bug-fix period we can use to clean up PUT/POST and such.
according to the html 4 spec [1], the only acceptable values for
form's method attribute are GET and POST. Also, in common practice,
PUT is used primarily in making ajax calls, or in accessing
programatic JSON or XML APIs, not in html forms. HTML5 does specify
PUT as a possible value for that attribute, but if you are planning on
improving your support of PUT, I have to believe that it is
*primarily* because you want to improve the ability to implement
programatic APIs in Django.
However, as I have detailed in my last two posts in this thread, the
current implementation, lacks any ajax/json/xml support. In those two
posts I have detailed possible modifications to the code that would
allow ajax support to be introduced. I think that Andrew Godwin's
remarks contribute quite a bit to that discussion, but no one else
responded yet. This is something that needs to be tangibly addressed.
I am worried that with the pace that things are going here that ajax
support may be ignored, or just pasted-on at the last minute.
Clearly, you see this as an important feature, or you wouldn't be
considering improving PUT support, and wouldn't be supporting the
DELETE verb. If ajax/json/xml are to be supported by the generic view
classes, however, some refactoring has to be made to the code.
[1] http://www.w3.org/TR/REC-html40/interact/forms.html#h-17.3
Luke Plant
> Ok - so to kick the process into the next phase, I've just created a
> Django branch in my bitbucket repo [1] to cover introducing this to
> trunk.
Russell - beware - I think bitbucket has managed to create a very broken
clone. I did a fork as well, and both of our repositories are missing
thousands of commits. I can do a pull from django/django and I then get
a complete repository which then seems to be compatible with
django/django. I've e-mailed the bitbucket folks, but have left my
clone as it is for now to allow them to work out what has gone wrong.
Ben Firshman
messages as spam or something.
Thanks to everyone who's helping push this forward. I would get stuck
in, but I'm bogged down with work at the moment.
A couple of things from the wiki page that need doing:
1) Test coverage probably isn't great. Everything seems to work when
I've used it in applications, but there's probably some stuff in there
that isn't thoroughly tested.
2) Support for ModelForms isn't quite the same as our current generic
views. For edit views, you can specify a model and the form is
optional.
Ben
On Oct 5, 3:43 pm, Jacob Kaplan-Moss <ja...@jacobian.org> wrote:
> On Tue, Oct 5, 2010 at 8:03 AM, Russell Keith-Magee
>