Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-11-02 Thread Ivan Sagalaev

On 11/02/2010 05:27 PM, Mikhail Korobov wrote:

I'm quite busy now and don't think I'll be able to make the patch
ready by 1.3 alpha 1.


Full feature freeze is expected only by the time of beta so I don't 
think it's absolutely necessary to push it before alpha 1.


Anyway since I care very much for the patch I can pick it up if you 
couldn't find a time to maintain it. Just drop me a line off-list in 
this case.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-11-02 Thread Mikhail Korobov
I'm quite busy now and don't think I'll be able to make the patch
ready by 1.3 alpha 1.

On 2 ноя, 20:06, Mikhail Korobov  wrote:
> Hi all.
>
> The new patch is attached to ticket 
> (seehttp://code.djangoproject.com/attachment/ticket/12816/render_shortcut...
> ). Docs are cumbersome (and incomplete), and a couple of middleware
> tests are missing.
>
> 1) Template response middleware is introduced. It is applied only for
> response instances that can be baked.
> 2) Exception is raised on content access or iteration for unbaked
> response.
> 3) 'force_bake' method is removed. 'bake' now behaves like old
> 'force_bake'.
> 4) Test client fixes are no longer needed and they are removed.
> 5) Some docs are added.
>
> Am we in hurry to get 'render' shortcut into 1.3 alpha 1 or it can be
> added later?
>
> On 31 окт, 00:49, Ivan Sagalaev  wrote:
>
>
>
> > On 10/30/2010 10:47 PM, SmileyChris wrote:
>
> > > It seems a simple enough proposal that trying to access the content
> > > property would raise an error until it is explicitly baked. Problem
> > > solved.
>
> > True. I seem to somehow missed it between the lines. Thanks! I'm +1 on
> > it by the way.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-11-02 Thread Mikhail Korobov
Hi all.

The new patch is attached to ticket (see
http://code.djangoproject.com/attachment/ticket/12816/render_shortcut.6.diff?format=raw
). Docs are cumbersome (and incomplete), and a couple of middleware
tests are missing.

1) Template response middleware is introduced. It is applied only for
response instances that can be baked.
2) Exception is raised on content access or iteration for unbaked
response.
3) 'force_bake' method is removed. 'bake' now behaves like old
'force_bake'.
4) Test client fixes are no longer needed and they are removed.
5) Some docs are added.

Am we in hurry to get 'render' shortcut into 1.3 alpha 1 or it can be
added later?

On 31 окт, 00:49, Ivan Sagalaev  wrote:
> On 10/30/2010 10:47 PM, SmileyChris wrote:
>
> > It seems a simple enough proposal that trying to access the content
> > property would raise an error until it is explicitly baked. Problem
> > solved.
>
> True. I seem to somehow missed it between the lines. Thanks! I'm +1 on
> it by the way.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-29 Thread Carl Meyer
On Oct 29, 2:04 pm, Ivan Sagalaev  wrote:
> Aha, I see the point now. On a second thought I think we can avoid this
> problem altogether by not passing actual response object into
> middleware. Instead we could pass just those bits that a middleware
> should care about: a template name and a context instance. The
> middleware then may (or even must) return new values for those that
> would be placed back into the response by the request handler.

Seems that a template-response-middleware might reasonably want to
look at some other response data (headers? or simply extra annotation
attributes?) in order to make decisions about what to do.

Carl

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-29 Thread Ivan Sagalaev

On 10/29/2010 09:58 AM, Russell Keith-Magee wrote:

I agree that it's important to treat people as grown ups. However,
this is something that is trivial to do by accident -- for example,
printing response.content would be an obvious debug step -- and it
will be a non-trivial thing to identify that this is the cause of your
problems.


Aha, I see the point now. On a second thought I think we can avoid this 
problem altogether by not passing actual response object into 
middleware. Instead we could pass just those bits that a middleware 
should care about: a template name and a context instance. The 
middleware then may (or even must) return new values for those that 
would be placed back into the response by the request handler.


Something still bothers me about it, but I can't invent any real 
objections myself.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-29 Thread Mikhail Korobov
On 29 окт, 10:09, Russell Keith-Magee  wrote:
>
> Ah - I wasn't aware there was a working implementation of this idea --
> did I miss a link somewhere?
>

No, there is no full working implementation. I'm talking about Ivan's
code snippet:

 response = get_response(...)
 if hastattr(response, 'force_bake'):
 # apply template response middleware
 response.force_bake()
 # apply normal response middleware

===

> If any template-response-middleware were to bake the
> response, subsequent template-reposnse-middlewares could potentially
> have problems, as any changes they make to context etc will be
> ignored.

Just realized that this is not correct. Changes won't be ignored but
the template will be rendered twice.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-29 Thread Russell Keith-Magee
On Fri, Oct 29, 2010 at 2:50 PM, Ivan Sagalaev
 wrote:
> Russel:
>>>
>>> Wouldn't it make sense to put a flag on the TemplateResponse
>>> that prohibits accidental baking?
>
> Mikhail:
>>
>> So maybe it will be better not to make bake/force_bake public so that
>> users won't be able to shoot themselves in the foot?
>
> I don't think it's doable at all. People still can call any method in as
> well as they can ignore or alter any protection flag. I believe it's
> sufficient to abide to the Python way here, write a proper documentation and
> treat people as grown-ups trusting them not to do bad things in their
> process_template_response.

I agree that it's important to treat people as grown ups. However,
this is something that is trivial to do by accident -- for example,
printing response.content would be an obvious debug step -- and it
will be a non-trivial thing to identify that this is the cause of your
problems.

Maybe an unconditional exception is a little strong, but some sort of
safety catch seems called for.

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-29 Thread Ivan Sagalaev

Russel:

Wouldn't it make sense to put a flag on the TemplateResponse
that prohibits accidental baking?


Mikhail:

So maybe it will be better not to make bake/force_bake public so that
users won't be able to shoot themselves in the foot?


I don't think it's doable at all. People still can call any method in as 
well as they can ignore or alter any protection flag. I believe it's 
sufficient to abide to the Python way here, write a proper documentation 
and treat people as grown-ups trusting them not to do bad things in 
their process_template_response.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-28 Thread Russell Keith-Magee
2010/10/29 Mikhail Korobov :
> Hi Russel,
>
> Thank you for your reviews and moving things on!
>
> On 29 окт, 07:35, Russell Keith-Magee  wrote:
>>
>> I like this idea -- it's is an elegant solution to the problem, and
>> avoids all the backwards compatibility issues I can think of.
>>
>> I have two comments:
>>
>> Firstly, there needs to be a shortcut for non-template responses. If
>> your response isn't a template response, there's no point putting it
>> through Template Reponse Middleware.
>>
>
> Ivan's code doesn't put non-template responses through template
> response middleware. Whenever the response is a template response is
> determined by the presence of 'force_bake' method.

Ah - I wasn't aware there was a working implementation of this idea --
did I miss a link somewhere?

>> Secondly, it seems to me like there may be some need for baking
>> protection here. If any template-response-middleware were to bake the
>> response, subsequent template-reposnse-middlewares could potentially
>> have problems, as any changes they make to context etc will be
>> ignored. Wouldn't it make sense to put a flag on the TemplateResponse
>> that prohibits accidental baking? That way the force_bake() that
>> happens between the template-response-middleware and the normal
>> response-middleware would be the guaranteed point at which the
>> template is writ large as content.
>>
>
> As I can see, users shouldn't bake responses not only in middlewares.
> They shouldn't bake responses anywhere in their code.
>
> The original TemplateResponse idea was not the same.

Agreed. This is a change from the original completely-lazy-evaluated
TemplateResponse idea, but I think it makes sense in terms of being
explicit and avoiding a wealth of potential bugs in implicit
evaluation.

> So maybe it will be better not to make bake/force_bake public so that
> users won't be able to shoot themselves in the foot? And maybe it'll
> be better even not to bake response magically on first content
> access?

That's essentially what I was suggesting -- that if a middleware or
decorator accidental accessed response.content before the end of
template-context-processor handling (when the explicit baking occurs),
it should raise an exception rather than implicitly baking the
response.

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-28 Thread Mikhail Korobov
Hi Russel,

Thank you for your reviews and moving things on!

On 29 окт, 07:35, Russell Keith-Magee  wrote:
>
> I like this idea -- it's is an elegant solution to the problem, and
> avoids all the backwards compatibility issues I can think of.
>
> I have two comments:
>
> Firstly, there needs to be a shortcut for non-template responses. If
> your response isn't a template response, there's no point putting it
> through Template Reponse Middleware.
>

Ivan's code doesn't put non-template responses through template
response middleware. Whenever the response is a template response is
determined by the presence of 'force_bake' method.

> Secondly, it seems to me like there may be some need for baking
> protection here. If any template-response-middleware were to bake the
> response, subsequent template-reposnse-middlewares could potentially
> have problems, as any changes they make to context etc will be
> ignored. Wouldn't it make sense to put a flag on the TemplateResponse
> that prohibits accidental baking? That way the force_bake() that
> happens between the template-response-middleware and the normal
> response-middleware would be the guaranteed point at which the
> template is writ large as content.
>

As I can see, users shouldn't bake responses not only in middlewares.
They shouldn't bake responses anywhere in their code.

The original TemplateResponse idea was not the same.

Original TemplateResponse was baked on first content access. Now
TemplateResponse should be baked exactly in one place: right after
template response middlewares. Other baking points seems to be error-
prone with this solution.

So maybe it will be better not to make bake/force_bake public so that
users won't be able to shoot themselves in the foot? And maybe it'll
be better even not to bake response magically on first content
access?

> Regarding #9886 and #14523 -- they're both RFC, and they're on my todo
> list of things to commit in the near future. I just need to find a few
> spare moments to give the patches a final review and commit.
>
> Yours,
> Russ Magee %-)

That's great!

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-28 Thread Russell Keith-Magee
On Thu, Oct 28, 2010 at 2:55 PM, Ivan Sagalaev
 wrote:
> On 10/27/2010 04:55 PM, Mikhail Korobov wrote:
>>
>> 1. 'Border' middleware is a backwards-compatible change, the
>> requirement to bake response in middleware isn't.
>>
>> The
>> difference is only that you propose to execute 'bake' in the end of
>> response cycle and I propose to execute it at the beginning of the
>> response cycle but to make this customizable (by changing the position
>> of the BakingMiddleware).
>
> I understand your points now, thanks. Two things bother me about 'border'
> middleware:
>
> - its semantics is a bit different than that of others middleware in the
> list in settings and this difference is not explicitly clear when looking at
> the list
>
> - it's bad to have a boilerplate code that people just have to put somewhere
>
> I've spent a night with a thought and now I think I can propose even better
> solution.
>
> We can introduce a new kind of middleware — "template response middleware"
> for lack of a better name. A user who wants to do something with a template
> response *before* it is baked has to write a middleware like this:
>
>    class ContextInjectionMiddleware:
>        def process_template_response(self, request, response):
>            # do something with response
>
> Request handling code would look like this then:
>
>    response = get_response(...)
>
>    if hastattr(response, 'force_bake'):
>        # apply template response middleware
>        response.force_bake()
>
>    # apply normal response middleware
>
> This way we:
>
> - are getting rid of force_bake in HttpResponse where it's a noop
> - maintain backward compatibility since response is baked before all
> currently written middleware
> - require explicitly named method to deal with a new concept
>
> What's not to like? :-)

I like this idea -- it's is an elegant solution to the problem, and
avoids all the backwards compatibility issues I can think of.

I have two comments:

Firstly, there needs to be a shortcut for non-template responses. If
your response isn't a template response, there's no point putting it
through Template Reponse Middleware.

Secondly, it seems to me like there may be some need for baking
protection here. If any template-response-middleware were to bake the
response, subsequent template-reposnse-middlewares could potentially
have problems, as any changes they make to context etc will be
ignored. Wouldn't it make sense to put a flag on the TemplateResponse
that prohibits accidental baking? That way the force_bake() that
happens between the template-response-middleware and the normal
response-middleware would be the guaranteed point at which the
template is writ large as content.

Regarding #9886 and #14523 -- they're both RFC, and they're on my todo
list of things to commit in the near future. I just need to find a few
spare moments to give the patches a final review and commit.

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-28 Thread Ivan Sagalaev

On 10/28/2010 12:24 PM, Mikhail Korobov wrote:

The request handling code have to be put into WSGIHandler and into
ModPythonHandler so I'll wait until the patch for 
http://code.djangoproject.com/ticket/9886
will be landed.


I'd say it's even worth to wait for 
http://code.djangoproject.com/ticket/14523 that moves response 
middleware application into the base code.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-28 Thread Mikhail Korobov
On 28 окт, 12:55, Ivan Sagalaev  wrote:
> On 10/27/2010 04:55 PM, Mikhail Korobov wrote:
>
> > 1. 'Border' middleware is a backwards-compatible change, the
> > requirement to bake response in middleware isn't.
>
> > The
> > difference is only that you propose to execute 'bake' in the end of
> > response cycle and I propose to execute it at the beginning of the
> > response cycle but to make this customizable (by changing the position
> > of the BakingMiddleware).
>
> I understand your points now, thanks. Two things bother me about
> 'border' middleware:
>
> - its semantics is a bit different than that of others middleware in the
> list in settings and this difference is not explicitly clear when
> looking at the list
>
> - it's bad to have a boilerplate code that people just have to put somewhere
>
> I've spent a night with a thought and now I think I can propose even
> better solution.
>
> We can introduce a new kind of middleware -- "template response
> middleware" for lack of a better name. A user who wants to do something
> with a template response *before* it is baked has to write a middleware
> like this:
>
>  class ContextInjectionMiddleware:
>  def process_template_response(self, request, response):
>  # do something with response
>
> Request handling code would look like this then:
>
>  response = get_response(...)
>
>  if hastattr(response, 'force_bake'):
>  # apply template response middleware
>  response.force_bake()
>
>  # apply normal response middleware
>
> This way we:
>
> - are getting rid of force_bake in HttpResponse where it's a noop
> - maintain backward compatibility since response is baked before all
> currently written middleware
> - require explicitly named method to deal with a new concept
>
> What's not to like? :-)

I like your solution.

The request handling code have to be put into WSGIHandler and into
ModPythonHandler so I'll wait until the patch for 
http://code.djangoproject.com/ticket/9886
will be landed.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-28 Thread Ivan Sagalaev

On 10/27/2010 04:55 PM, Mikhail Korobov wrote:

1. 'Border' middleware is a backwards-compatible change, the
requirement to bake response in middleware isn't.

The
difference is only that you propose to execute 'bake' in the end of
response cycle and I propose to execute it at the beginning of the
response cycle but to make this customizable (by changing the position
of the BakingMiddleware).


I understand your points now, thanks. Two things bother me about 
'border' middleware:


- its semantics is a bit different than that of others middleware in the 
list in settings and this difference is not explicitly clear when 
looking at the list


- it's bad to have a boilerplate code that people just have to put somewhere

I've spent a night with a thought and now I think I can propose even 
better solution.


We can introduce a new kind of middleware — "template response 
middleware" for lack of a better name. A user who wants to do something 
with a template response *before* it is baked has to write a middleware 
like this:


class ContextInjectionMiddleware:
def process_template_response(self, request, response):
# do something with response

Request handling code would look like this then:

response = get_response(...)

if hastattr(response, 'force_bake'):
# apply template response middleware
response.force_bake()

# apply normal response middleware

This way we:

- are getting rid of force_bake in HttpResponse where it's a noop
- maintain backward compatibility since response is baked before all 
currently written middleware

- require explicitly named method to deal with a new concept

What's not to like? :-)

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-27 Thread Mikhail Korobov
Hmm, and now I don's see a use case for the 'force_bake' (and maybe
even public 'bake' method) method if BakingMiddleware is implemented.
With BakingMiddleware there is exactly one place where response should
be baked and user's code shouldn't be calling 'force_bake' and even
'bake' on responses.

On 27 окт, 19:55, Mikhail Korobov  wrote:
> Hi Ivan,
>
> Let me explain why I prefer 'border' middleware way (that is
> implemented) over explicit baking in messages middleware (that was
> implemented but then replaced with 'border' middleware).
>
> 1. 'Border' middleware is a backwards-compatible change, the
> requirement to bake response in middleware isn't.
>
> 2. With BakingMiddleware it is clear what middlewares expect responses
> to be baked and what expect responses to be unbaked. This provides
> practical benefits. If there is no BakingMiddleware and some
> middleware wants to change template or context it must call
> 'force_bake' (not just 'bake') to make sure the changes will apply. If
> there is an another middleware that want to change something (template
> or context) then template will be rendered several times. With
> BakingMiddleware the contract is a bit different. Template will be
> rendered exactly 1 time in this case because middleware can assume
> that the response was not baked.
>
> 3. The code behind BakingMiddleware should be executed anyway in order
> to prevent template rendering outside try-catch statements. The
> difference is only that you propose to execute 'bake' in the end of
> response cycle and I propose to execute it at the beginning of the
> response cycle but to make this customizable (by changing the position
> of the BakingMiddleware).
>
> On 27 окт, 13:33, Ivan Sagalaev  wrote:
>
>
>
> > On 10/25/2010 04:33 PM, Russell Keith-Magee wrote:
>
> > >   * The problem with messages is a big one -- probably even a
> > > show-stopper if we can't find a way to reconcile the general use case
> > > that it represents (i.e., we don't just need a fix for
> > > contrib.messages  -- we need to explain how/why the problem exists,
> > > and provide a consistent approach for analogous problems)
>
> > I apologize in advance if I missed some important bits of the
> > conversation I'm on vacation right now :-).
>
> > Why can't we teach messages middleware to just explicitly bake a
> > response and call it a proper solution? By adding `force_bake` to the
> > HttpResponse class itself we effectively declare that *any* HttpResponse
> > can be lazy and middleware that expects some side-effects from a
> > response has to bake it.
>
> > Sure it must be documented that middleware forcing response baking
> > should work after the ones that don't.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-27 Thread Mikhail Korobov
Hi Ivan,

Let me explain why I prefer 'border' middleware way (that is
implemented) over explicit baking in messages middleware (that was
implemented but then replaced with 'border' middleware).

1. 'Border' middleware is a backwards-compatible change, the
requirement to bake response in middleware isn't.

2. With BakingMiddleware it is clear what middlewares expect responses
to be baked and what expect responses to be unbaked. This provides
practical benefits. If there is no BakingMiddleware and some
middleware wants to change template or context it must call
'force_bake' (not just 'bake') to make sure the changes will apply. If
there is an another middleware that want to change something (template
or context) then template will be rendered several times. With
BakingMiddleware the contract is a bit different. Template will be
rendered exactly 1 time in this case because middleware can assume
that the response was not baked.

3. The code behind BakingMiddleware should be executed anyway in order
to prevent template rendering outside try-catch statements. The
difference is only that you propose to execute 'bake' in the end of
response cycle and I propose to execute it at the beginning of the
response cycle but to make this customizable (by changing the position
of the BakingMiddleware).

On 27 окт, 13:33, Ivan Sagalaev  wrote:
> On 10/25/2010 04:33 PM, Russell Keith-Magee wrote:
>
> >   * The problem with messages is a big one -- probably even a
> > show-stopper if we can't find a way to reconcile the general use case
> > that it represents (i.e., we don't just need a fix for
> > contrib.messages  -- we need to explain how/why the problem exists,
> > and provide a consistent approach for analogous problems)
>
> I apologize in advance if I missed some important bits of the
> conversation I'm on vacation right now :-).
>
> Why can't we teach messages middleware to just explicitly bake a
> response and call it a proper solution? By adding `force_bake` to the
> HttpResponse class itself we effectively declare that *any* HttpResponse
> can be lazy and middleware that expects some side-effects from a
> response has to bake it.
>
> Sure it must be documented that middleware forcing response baking
> should work after the ones that don't.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-27 Thread Ivan Sagalaev

On 10/25/2010 04:33 PM, Russell Keith-Magee wrote:

  * The problem with messages is a big one -- probably even a
show-stopper if we can't find a way to reconcile the general use case
that it represents (i.e., we don't just need a fix for
contrib.messages  -- we need to explain how/why the problem exists,
and provide a consistent approach for analogous problems)


I apologize in advance if I missed some important bits of the 
conversation — I'm on vacation right now :-).


Why can't we teach messages middleware to just explicitly bake a 
response and call it a proper solution? By adding `force_bake` to the 
HttpResponse class itself we effectively declare that *any* HttpResponse 
can be lazy and middleware that expects some side-effects from a 
response has to bake it.


Sure it must be documented that middleware forcing response baking 
should work after the ones that don't.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-25 Thread Mikhail Korobov
I propose the following solution for middleware problem:

1. Introduce the BakingMiddleware
(django.template.response.BakingMiddleware or
django.template.middleware.BakingMiddleware?). This middleware bakes
the response using .bake() method.
2. Put this middleware as last middleware in default settings.py (with
a blank line above).
3. The ugly bit: if there is no BakingMiddleware in
settings.MIDDLEWARE_CLASSES then assume that it is the last
middleware.
django.core.handlers.base.BaseHandler should be patched for that.
Warning may also be emitted if there is no explicit BakingMiddleware
in settings.py so that this ugly bit can be removed in future.
4. raise ImproperlyConfigured exception if unbaked response is
received by messages middleware

This way changes will be backwards-compatible on middleware level
because middlewares will only receive baked responses by default. No
existing middlewares make use of TemplateResponse and things will work
as usual with default setup.

Cool new response middlewares that makes use of TemplateResponse (e.g.
do some caching or paginating on context objects) can be put after the
BakingMiddleware so they will be able mess with an unbaked response if
it is available.

Implementation (no tests and docs): 
http://bitbucket.org/kmike/django/changeset/68571aa0e5a3

On 25 окт, 20:36, Mikhail Korobov  wrote:
> contrib.messages middleware was broken because it relies on something
> that should happen on template rendering (iteration over the messages
> in this case) and don't access response content directly.
>
> I was about to introduce 'BakingMiddleware' - the middleware that
> bakes the response explicitly. It can be a border between middlewares
> with rendered templates as requirement and middlewares without this
> requirement. But we can't inject this middleware in backwards-
> compatible way to user's code so I just fixed the contrib.messages
> middleware. I still like the idea but don't know if we can afford 'add
> BakingMiddleware before the MessagesMiddleware' note in upgrade docs.
>
> This is all quite similar to problems django have with streaming http
> responses 
> (seehttp://groups.google.com/group/django-developers/browse_thread/thread...)
> and maybe there are some ideas from that thread which may be useful.
>
> On 25 окт, 19:33, Russell Keith-Magee  wrote:
>
>
>
> > 2010/10/25 Mikhail Korobov :
>
> > > Sorry for massive email spam on this list :)
>
> > > I came up with even more naive implementation of
> > > TemplateResponseMixin:http://bitbucket.org/kmike/django/src/a3e242ca7b4b/django/views/gener...
>
> > > response.template_name will contain a list of names and there is
> > > (almost) no code duplication between TemplateResponse and
> > > TemplateResponseMixin with this implementation. Custom template
> > > loading and context instantiation go to TemplateResponse subclasses.
>
> > This is starting to look good to me; here are some comments, going
> > back a couple of messages:
>
> >  * Yes, you've got the right idea with regards to the role played by
> > the various TemplateResponseMixin methods
>
> >  * It seems reasonable to me that assertTemplateUsed would require
> > some baking, and yes, that should be happening at the test client
> > before template rendering signals are disconnected.
>
> >  * The problem with messages is a big one -- probably even a
> > show-stopper if we can't find a way to reconcile the general use case
> > that it represents (i.e., we don't just need a fix for
> > contrib.messages  -- we need to explain how/why the problem exists,
> > and provide a consistent approach for analogous problems)
>
> >  * Following the convention of the rest of the API, the call to
> > get_template_names() should be internal to get_response(), not passed
> > in as an argument.
>
> >  * I'm not entirely convinced that get_response() is needed now. As
> > your implementation currently stands, render_to_response() is just a
> > call to get_response() -- which suggests that the extra level of
> > indirection isn't needed.
>
> > Backing up this position -- most of the flexibility that
> > TemplateResponseMixin has is to enable the easy integration of
> > different rendering engines and contexts; those API points are now
> > provided by TemplateResponse, so there isn't any need to preserve them
> > in the mixin. If you want to use a different loader, template
> > renderer, context instance, etc, you subclass TemplateResponse.
>
> > So - revised source code:
>
> > class TemplateResponseMixin(object):
> >     """
> >     A mixin that can be used to render a template.
> >     """
> >     template_name = None
> >     template_response_class = TemplateResponse
>
> >     def render_to_response(self, context):
> >         """
> >         Returns a response with a template rendered with the given context.
> >         """
> >         return self.template_response_class(
> >             

Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-25 Thread Mikhail Korobov
contrib.messages middleware was broken because it relies on something
that should happen on template rendering (iteration over the messages
in this case) and don't access response content directly.

I was about to introduce 'BakingMiddleware' - the middleware that
bakes the response explicitly. It can be a border between middlewares
with rendered templates as requirement and middlewares without this
requirement. But we can't inject this middleware in backwards-
compatible way to user's code so I just fixed the contrib.messages
middleware. I still like the idea but don't know if we can afford 'add
BakingMiddleware before the MessagesMiddleware' note in upgrade docs.

This is all quite similar to problems django have with streaming http
responses (see
http://groups.google.com/group/django-developers/browse_thread/thread/9dc1bb93eed77987/c61c1b8d5426c1cb?lnk=gst=http+content#c61c1b8d5426c1cb)
and maybe there are some ideas from that thread which may be useful.

On 25 окт, 19:33, Russell Keith-Magee  wrote:
> 2010/10/25 Mikhail Korobov :
>
> > Sorry for massive email spam on this list :)
>
> > I came up with even more naive implementation of
> > TemplateResponseMixin:http://bitbucket.org/kmike/django/src/a3e242ca7b4b/django/views/gener...
>
> > response.template_name will contain a list of names and there is
> > (almost) no code duplication between TemplateResponse and
> > TemplateResponseMixin with this implementation. Custom template
> > loading and context instantiation go to TemplateResponse subclasses.
>
> This is starting to look good to me; here are some comments, going
> back a couple of messages:
>
>  * Yes, you've got the right idea with regards to the role played by
> the various TemplateResponseMixin methods
>
>  * It seems reasonable to me that assertTemplateUsed would require
> some baking, and yes, that should be happening at the test client
> before template rendering signals are disconnected.
>
>  * The problem with messages is a big one -- probably even a
> show-stopper if we can't find a way to reconcile the general use case
> that it represents (i.e., we don't just need a fix for
> contrib.messages  -- we need to explain how/why the problem exists,
> and provide a consistent approach for analogous problems)
>
>  * Following the convention of the rest of the API, the call to
> get_template_names() should be internal to get_response(), not passed
> in as an argument.
>
>  * I'm not entirely convinced that get_response() is needed now. As
> your implementation currently stands, render_to_response() is just a
> call to get_response() -- which suggests that the extra level of
> indirection isn't needed.
>
> Backing up this position -- most of the flexibility that
> TemplateResponseMixin has is to enable the easy integration of
> different rendering engines and contexts; those API points are now
> provided by TemplateResponse, so there isn't any need to preserve them
> in the mixin. If you want to use a different loader, template
> renderer, context instance, etc, you subclass TemplateResponse.
>
> So - revised source code:
>
> class TemplateResponseMixin(object):
>     """
>     A mixin that can be used to render a template.
>     """
>     template_name = None
>     template_response_class = TemplateResponse
>
>     def render_to_response(self, context):
>         """
>         Returns a response with a template rendered with the given context.
>         """
>         return self.template_response_class(
>             request=self.request,
>             template=self.get_template_names(),
>             context=context,
>             **response_kwargs
>         )
>
>     def get_template_names(self):
>         """
>         Returns a list of template names to be used for the request. Must 
> return
>         a list. May not be called if render_to_response is overridden.
>         """
>         if self.template_name is None:
>             return []
>         else:
>             return [self.template_name]
>
> However, all this is a moot point if we can't find a fix for the
> contrib.messages problem.
>
> 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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-25 Thread Mikhail Korobov
Sorry for massive email spam on this list :)

I came up with even more naive implementation of
TemplateResponseMixin: 
http://bitbucket.org/kmike/django/src/a3e242ca7b4b/django/views/generic/base.py#cl-87

response.template_name will contain a list of names and there is
(almost) no code duplication between TemplateResponse and
TemplateResponseMixin with this implementation. Custom template
loading and context instantiation go to TemplateResponse subclasses.

On 24 окт, 17:32, Mikhail Korobov  wrote:
> new changes (integration with generic views, test client and messages
> middleware fixes):http://bitbucket.org/kmike/django/overview
>
> Yet another gotchas:
>
> - response.template_name for generic views will contain Template
> instance, not template names, so response.template_name is quite
> misleading. The better name ('template') is already taken by test
> client (but it is deprecated in 1.3). I can't find a good solution so
> leave the 'template_name' for now.
>
> - hasattr(response, 'bake') and callable(response.bake) checks are
> ugly. The alternative is to provide HttpResponse.bake method but this
> way HttpResponse will be aware of TemplateResponse and it doesn't seem
> clean for me.
>
> On 24 окт, 02:14, Mikhail Korobov  wrote:
>
>
>
> > Yes, you're right and I was wrong, the messages middleware doesn't
> > return response as-is. I'll take a look.
>
> > As for tests, response.context and response.templates are not
> > available for TemplateResponse instances before they are baked so test
> > client should be patched to explicitly bake the response. There is
> > response.template_context and response.template_name but their
> > semantics differ.
>
> > On 24 окт, 01:52, SmileyChris  wrote:
>
> > > The points were just off the top of my head from memory, when I get
> > > back to work I'll have a look to see what the actual cases are.
>
> > > Regarding the messages middleware, I *know* there's a problem. A
> > > message won't be marked as "read", since the template hasn't iterated
> > > the messages object by the time the middleware is triggered
>
> > > On Oct 23, 8:35 am, Mikhail Korobov  wrote:
>
> > > > Hi Chris,
>
> > > > I don't see anything harmful neither in
> > > > django.contrib.messages.middleware.MessageMiddleware nor in
> > > > django.test.testcases.assertContains.
> > > > Messages middleware passes response as-is and assertContains reads
> > > > 'content' attribute and thus forces the baking.
>
> > > > at lest the following test case forks fine for me:
>
> > > > class AssertTestCase(TestCase):
> > > >     def test_assert_contains(self):
> > > >         request = RequestFactory().get('/')
> > > >         template = Template('foo')
> > > >         response = TemplateResponse(request, template)
> > > >         self.assertContains(response, 'oo')
>
> > > > Can you please provide more details?
>
> > > > On 23 окт, 00:21, SmileyChris  wrote:
>
> > > > > In my use of TemplateResponse in a real project, we encountered two
> > > > > gotchas that I can think of off the top of my head:
>
> > > > > 1. You need to explicitly bake the response if you are testing using
> > > > > assertContains
> > > > > 2. You need to explicitly bake the response before the
> > > > > contrib.messages middleware
>
> > > > > On Oct 23, 1:32 am, Russell Keith-Magee 
> > > > > wrote:
>
> > > > > > On Fri, Oct 22, 2010 at 7:32 PM, Mikhail Korobov 
> > > > > >  wrote:
> > > > > > > Russell's comments were helpful in discovering the edge case.
> > > > > > > _set_content behaves differently for baked and non-baked 
> > > > > > > responses:
>
> > > > > > > response = render(request, Template('foo'))
> > > > > > > response.content = 'bar'
> > > > > > > print response.content    # 'foo'
> > > > > > > response.content = 'baz'
> > > > > > > print response.content    # 'baz'
>
> > > > > > > This is confusing so I think responses should be marked as baked 
> > > > > > > in
> > > > > > > _set_content, not in force_bake.
>
> > > > > > > The patch that should resolve this concern and Russell's concerns
> > > > > > > regarding the 
> > > > > > > tests:http://bitbucket.org/kmike/django/changeset/00f8be464749
>
> > > > > > > I'll take a look at docs and generic views integration later.
>
> > > > > > > Should new generic views return TemplateResponse by default?
>
> > > > > > I would have thought so. Is there a compelling reason why CBV's
> > > > > > shouldn't return a TemplateResponse?
>
> > > > > > 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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 

Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-24 Thread Mikhail Korobov
new changes (integration with generic views, test client and messages
middleware fixes): http://bitbucket.org/kmike/django/overview

Yet another gotchas:

- response.template_name for generic views will contain Template
instance, not template names, so response.template_name is quite
misleading. The better name ('template') is already taken by test
client (but it is deprecated in 1.3). I can't find a good solution so
leave the 'template_name' for now.

- hasattr(response, 'bake') and callable(response.bake) checks are
ugly. The alternative is to provide HttpResponse.bake method but this
way HttpResponse will be aware of TemplateResponse and it doesn't seem
clean for me.

On 24 окт, 02:14, Mikhail Korobov  wrote:
> Yes, you're right and I was wrong, the messages middleware doesn't
> return response as-is. I'll take a look.
>
> As for tests, response.context and response.templates are not
> available for TemplateResponse instances before they are baked so test
> client should be patched to explicitly bake the response. There is
> response.template_context and response.template_name but their
> semantics differ.
>
> On 24 окт, 01:52, SmileyChris  wrote:
>
>
>
> > The points were just off the top of my head from memory, when I get
> > back to work I'll have a look to see what the actual cases are.
>
> > Regarding the messages middleware, I *know* there's a problem. A
> > message won't be marked as "read", since the template hasn't iterated
> > the messages object by the time the middleware is triggered
>
> > On Oct 23, 8:35 am, Mikhail Korobov  wrote:
>
> > > Hi Chris,
>
> > > I don't see anything harmful neither in
> > > django.contrib.messages.middleware.MessageMiddleware nor in
> > > django.test.testcases.assertContains.
> > > Messages middleware passes response as-is and assertContains reads
> > > 'content' attribute and thus forces the baking.
>
> > > at lest the following test case forks fine for me:
>
> > > class AssertTestCase(TestCase):
> > >     def test_assert_contains(self):
> > >         request = RequestFactory().get('/')
> > >         template = Template('foo')
> > >         response = TemplateResponse(request, template)
> > >         self.assertContains(response, 'oo')
>
> > > Can you please provide more details?
>
> > > On 23 окт, 00:21, SmileyChris  wrote:
>
> > > > In my use of TemplateResponse in a real project, we encountered two
> > > > gotchas that I can think of off the top of my head:
>
> > > > 1. You need to explicitly bake the response if you are testing using
> > > > assertContains
> > > > 2. You need to explicitly bake the response before the
> > > > contrib.messages middleware
>
> > > > On Oct 23, 1:32 am, Russell Keith-Magee 
> > > > wrote:
>
> > > > > On Fri, Oct 22, 2010 at 7:32 PM, Mikhail Korobov 
> > > > >  wrote:
> > > > > > Russell's comments were helpful in discovering the edge case.
> > > > > > _set_content behaves differently for baked and non-baked responses:
>
> > > > > > response = render(request, Template('foo'))
> > > > > > response.content = 'bar'
> > > > > > print response.content    # 'foo'
> > > > > > response.content = 'baz'
> > > > > > print response.content    # 'baz'
>
> > > > > > This is confusing so I think responses should be marked as baked in
> > > > > > _set_content, not in force_bake.
>
> > > > > > The patch that should resolve this concern and Russell's concerns
> > > > > > regarding the 
> > > > > > tests:http://bitbucket.org/kmike/django/changeset/00f8be464749
>
> > > > > > I'll take a look at docs and generic views integration later.
>
> > > > > > Should new generic views return TemplateResponse by default?
>
> > > > > I would have thought so. Is there a compelling reason why CBV's
> > > > > shouldn't return a TemplateResponse?
>
> > > > > 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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-23 Thread Mikhail Korobov
Yes, you're right and I was wrong, the messages middleware doesn't
return response as-is. I'll take a look.

As for tests, response.context and response.templates are not
available for TemplateResponse instances before they are baked so test
client should be patched to explicitly bake the response. There is
response.template_context and response.template_name but their
semantics differ.


On 24 окт, 01:52, SmileyChris  wrote:
> The points were just off the top of my head from memory, when I get
> back to work I'll have a look to see what the actual cases are.
>
> Regarding the messages middleware, I *know* there's a problem. A
> message won't be marked as "read", since the template hasn't iterated
> the messages object by the time the middleware is triggered
>
> On Oct 23, 8:35 am, Mikhail Korobov  wrote:
>
>
>
> > Hi Chris,
>
> > I don't see anything harmful neither in
> > django.contrib.messages.middleware.MessageMiddleware nor in
> > django.test.testcases.assertContains.
> > Messages middleware passes response as-is and assertContains reads
> > 'content' attribute and thus forces the baking.
>
> > at lest the following test case forks fine for me:
>
> > class AssertTestCase(TestCase):
> >     def test_assert_contains(self):
> >         request = RequestFactory().get('/')
> >         template = Template('foo')
> >         response = TemplateResponse(request, template)
> >         self.assertContains(response, 'oo')
>
> > Can you please provide more details?
>
> > On 23 окт, 00:21, SmileyChris  wrote:
>
> > > In my use of TemplateResponse in a real project, we encountered two
> > > gotchas that I can think of off the top of my head:
>
> > > 1. You need to explicitly bake the response if you are testing using
> > > assertContains
> > > 2. You need to explicitly bake the response before the
> > > contrib.messages middleware
>
> > > On Oct 23, 1:32 am, Russell Keith-Magee 
> > > wrote:
>
> > > > On Fri, Oct 22, 2010 at 7:32 PM, Mikhail Korobov 
> > > >  wrote:
> > > > > Russell's comments were helpful in discovering the edge case.
> > > > > _set_content behaves differently for baked and non-baked responses:
>
> > > > > response = render(request, Template('foo'))
> > > > > response.content = 'bar'
> > > > > print response.content    # 'foo'
> > > > > response.content = 'baz'
> > > > > print response.content    # 'baz'
>
> > > > > This is confusing so I think responses should be marked as baked in
> > > > > _set_content, not in force_bake.
>
> > > > > The patch that should resolve this concern and Russell's concerns
> > > > > regarding the 
> > > > > tests:http://bitbucket.org/kmike/django/changeset/00f8be464749
>
> > > > > I'll take a look at docs and generic views integration later.
>
> > > > > Should new generic views return TemplateResponse by default?
>
> > > > I would have thought so. Is there a compelling reason why CBV's
> > > > shouldn't return a TemplateResponse?
>
> > > > 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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-23 Thread Mikhail Korobov
Hi all again!

I've done some research on generic views integration and think that
TemplateResponseMixin should be refactored in order to use
TemplateResponse (SimpleTemplateResponse actually) because it
currently assumes that template must be rendered before the response
is returned.

'render_template' hook doesn't fit lazy-rendered responses and
'get_response' == 'render_to_response' if there is no
'render_template' hook. I propose to eliminate 'get_response' and
'render_template' hooks. These hooks are not used by django itself.

get_response's purpose was to make it easy to override response class.
Because render_to_response now returns response itself it is natural
to override render_to_response instead of get_response.

render_template's purpose was to change a way template is rendered. I
don't know what is the exact use case. If one wants to use e.g. jinja2
template engine then he should override get_template method so it will
return jinja.Template instead of django.Template (both have a 'render'
method).

Am I understand their purposes properly?

Anyway, one can override render_to_response hook and return any
HttpResponse subclass using any template rendering logic.

Here is a chart to make things clear:
http://www.lucidchart.com/publicSegments/view/4cc313b1-9478-4985-9870-2a190afcbe04

Some other gotchas:
- TemplateResponseMixin.render_to_response should possibly be renamed
to 'TemplateResponseMixin.render' for consistency.
- django.test.TestCase.assertTemplateUsed should possibly bake the
response explicitly because TemplateResponses are lazy.

I need core developers' blessing.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-23 Thread Mikhail Korobov
On 22 окт, 18:10, Łukasz Rekucki  wrote:
> On 22 October 2010 03:59, Russell Keith-Magee  wrote:
> > 2010/10/21 Łukasz Rekucki :
> >> Both render_to_response() and direct_to_template() have one very
> >> annoying flaw:http://code.djangoproject.com/ticket/12669. Please add
> >> a "response_class" keyword to your render() function ;). Thanks!
>
> > Is this addressed by the status_code argument to TemplateResponse? i.e.,
>
> > return TemplateResponse(request, template, context, status_code=403)
>
> > We could also include some constants to make things a little prettier:
>
> > return TemplateResponse(request, template, context, status_code=FORBIDDEN)
>
> > Would that satisfy your use case for #12669?
>
> Yes, it does, but constants are a must have, imho.
>

I think we shouldn't include constants in django itself because they
are already available:

>>> import httplib
>>> httplib.FORBIDDEN
403

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-22 Thread Mikhail Korobov
Hi Chris,

I don't see anything harmful neither in
django.contrib.messages.middleware.MessageMiddleware nor in
django.test.testcases.assertContains.
Messages middleware passes response as-is and assertContains reads
'content' attribute and thus forces the baking.

at lest the following test case forks fine for me:

class AssertTestCase(TestCase):
def test_assert_contains(self):
request = RequestFactory().get('/')
template = Template('foo')
response = TemplateResponse(request, template)
self.assertContains(response, 'oo')

Can you please provide more details?

On 23 окт, 00:21, SmileyChris  wrote:
> In my use of TemplateResponse in a real project, we encountered two
> gotchas that I can think of off the top of my head:
>
> 1. You need to explicitly bake the response if you are testing using
> assertContains
> 2. You need to explicitly bake the response before the
> contrib.messages middleware
>
> On Oct 23, 1:32 am, Russell Keith-Magee 
> wrote:
>
>
>
> > On Fri, Oct 22, 2010 at 7:32 PM, Mikhail Korobov  
> > wrote:
> > > Russell's comments were helpful in discovering the edge case.
> > > _set_content behaves differently for baked and non-baked responses:
>
> > > response = render(request, Template('foo'))
> > > response.content = 'bar'
> > > print response.content    # 'foo'
> > > response.content = 'baz'
> > > print response.content    # 'baz'
>
> > > This is confusing so I think responses should be marked as baked in
> > > _set_content, not in force_bake.
>
> > > The patch that should resolve this concern and Russell's concerns
> > > regarding the 
> > > tests:http://bitbucket.org/kmike/django/changeset/00f8be464749
>
> > > I'll take a look at docs and generic views integration later.
>
> > > Should new generic views return TemplateResponse by default?
>
> > I would have thought so. Is there a compelling reason why CBV's
> > shouldn't return a TemplateResponse?
>
> > 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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-22 Thread Russell Keith-Magee
On Fri, Oct 22, 2010 at 7:32 PM, Mikhail Korobov  wrote:
> Russell's comments were helpful in discovering the edge case.
> _set_content behaves differently for baked and non-baked responses:
>
> response = render(request, Template('foo'))
> response.content = 'bar'
> print response.content    # 'foo'
> response.content = 'baz'
> print response.content    # 'baz'
>
> This is confusing so I think responses should be marked as baked in
> _set_content, not in force_bake.
>
> The patch that should resolve this concern and Russell's concerns
> regarding the tests: http://bitbucket.org/kmike/django/changeset/00f8be464749
>
> I'll take a look at docs and generic views integration later.
>
> Should new generic views return TemplateResponse by default?

I would have thought so. Is there a compelling reason why CBV's
shouldn't return a TemplateResponse?

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-22 Thread Ivan Sagalaev

Hi Russel,

On 10/22/2010 05:20 AM, Russell Keith-Magee wrote:

Jacob has already marked #9886 RFC, and on first inspection, the patch
looks good to me too; I want to have a closer look before I commit,
though. If you want to proceed assuming that #9886 will be committed
(i.e., make the fix for #14523 have #9866 as a prerequisite), I doubt
it would be wasted effort.


I didn't feel it would be wasted :-). I was going to start doing the new 
patch on top of my local bzr branch with #9886 applied (arent' DVCSes 
awesome?). What I meant is that I didn't want to actually attach a diff 
file to the ticket yet because it has chances of not being applicable 
against trunk.



Regarding the second problem you describe -- unless I'm mistaken,
Simon's TemplateResponse addresses this with his "baking" concept; a
response is baked the first time it is iterated or otherwise
evaluated.


The problem is that this iteration may (and does) happen after response 
is returned from the handler — and hence outside the `try .. except`. 
The point is to call baking explicitly right before the return.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Russell Keith-Magee
2010/10/21 Łukasz Rekucki :
> On 20 October 2010 21:57, Jacob Kaplan-Moss  wrote:
>> 2010/10/20 Mikhail Korobov :
>>> There is an unresolved question in the ticket: "The only hesitation is
>>> the relationship with #12815; we should resolve that decision before
>>> committing anything for this ticket."
>>>
>>> #12815 is about introducing TemplateResponse. Is the patch with
>>> 'render' shortcut returning just HttpResponse acceptable? I think that
>>> TemplateResponse is less useful after class-based views make their way
>>> into trunk so 'render' shortcut shouldn't bother returning
>>> TemplateResponse. There are ways to reuse view logic (and change view
>>> context in particular) now and TemplateResponse (which was a good
>>> addition to django 1.2/1.1/1.0) seems to only complicate things in
>>> django 1.3.
>>
>> I agree completely with this reasoning - just render returning an
>> HttpResponse is fine, I think.
>
> Both render_to_response() and direct_to_template() have one very
> annoying flaw: http://code.djangoproject.com/ticket/12669. Please add
> a "response_class" keyword to your render() function ;). Thanks!

Is this addressed by the status_code argument to TemplateResponse? i.e.,

return TemplateResponse(request, template, context, status_code=403)

We could also include some constants to make things a little prettier:

return TemplateResponse(request, template, context, status_code=FORBIDDEN)

Would that satisfy your use case for #12669?

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Russell Keith-Magee
On Fri, Oct 22, 2010 at 8:19 AM, Mikhail Korobov  wrote:
> Patch is ready for review: 
> http://bitbucket.org/kmike/django/changeset/37d977574923
>
> This is the TempleteResponse by Simon Willison with tests and minor
> tweaks.
>
> Notes:
>
> - TemplateResponse and SimpleTemplateResponse reside in
> django.template.response
> - django/shortcuts/__init__.py used to have extra spaces. They are
> removed.
> - *args and **kwargs in TemplateResponse and SimpleTemplateResponse
> are gone in order to provide more help for IDEs
> - 'content' argument is not allowed for TemplateResponse and
> SimpleTemplateResponse now
> - context can be omitted now, the only required argument for
> SimpleTemplateResponse is the template; request and template are
> required for TemplateResponse
> - 'response_class' is not implemented but the status code can be
> passed as 'status' parameter.
> - _set_content doesn't return a value now

This is starting to look quite good. Some quick review comments:

 * Integration with generic views. I would have thought that
TemplateResponseMixin would be a natural place to be using a
TemplateResponse.

 * Tests can't rely on assumed context processors, as the comment in
the TemplateResponse tests states. See the flatpages views tests for
an example of how to work around this sort of problem.

 * MOAR TESTS!1!! :-) There are a couple of API points that aren't
tested, such as set_content(), force_bake() and iteration over content
as a baking trigger. There might be a couple of others; these are just
the ones that jumped out at me.

 * Docs! I appreciate that writing docs is premature when we're still
fiddling with details, but I think the core API is getting close, so
now would be a good time to at least put in some stub documentation
indicating what needs to be filled out later.

Yours,
Russ Magee %-)

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Russell Keith-Magee
On Fri, Oct 22, 2010 at 12:34 AM, Ivan Sagalaev
 wrote:
> On 10/21/2010 03:22 PM, Ivan Sagalaev wrote:
>>
>> On 10/21/2010 11:49 AM, Mikhail Korobov wrote:
>>>
>>> 2. Does TemplateResponse allow pretty exception pages or not? Is Ben's
>>> issue resolved?
>>
>> I'll look into it this evening (MSD).
>
> So I did.
>
> There are actually two problems:
>
> - Exceptions in response middleware are indeed happen outside of the
> request's `try .. except` block. This is a problem by itself[1] and I'd be
> happy to fix it after ticket 9886[2] is committed. It's another refactoring
> of core request code so I don't want to mess things up doing one patch on
> top of another :-).
>
> - Non-pretty plain text tracebacks can be caused not only be middleware but
> also by any error occurring during template rendering. Because all this
> happen *after* request was returned to the web server handler and is being
> iterated over.
>
> This second problem can be easily fixed by introducing a method for explicit
> evaluation of the content: `evaluate()` or `force_content()` that will be a
> noop for any HttpRespone class except the TemplateResponse. The method will
> be called by the request handler right before returning the response.
>
> Sounds good?

Hi Ivan and Mikhail,

Just so you don't get the feeling that you're just discussing this
between yourselves -- I'm historically on record of being in favor of
the render() shortcut and the TemplateResponse(), and my position
hasn't changed recently.  This is exactly the sort of problem that I
want to target for 1.3 (especially since we're now tracking 6 closely
related tickets -- #9081, #9886, #12815, #12816, #12669 and #14523 --
making this a sweet spot for attention).

Jacob has already marked #9886 RFC, and on first inspection, the patch
looks good to me too; I want to have a closer look before I commit,
though. If you want to proceed assuming that #9886 will be committed
(i.e., make the fix for #14523 have #9866 as a prerequisite), I doubt
it would be wasted effort.

Regarding the second problem you describe -- unless I'm mistaken,
Simon's TemplateResponse addresses this with his "baking" concept; a
response is baked the first time it is iterated or otherwise
evaluated.

I have a couple of non-Django things to attend to over the next week
or so; but once those are sorted out, I should be able to take a
detailed look at any patches you have prepared.

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-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Mikhail Korobov
Patch is ready for review: 
http://bitbucket.org/kmike/django/changeset/37d977574923

This is the TempleteResponse by Simon Willison with tests and minor
tweaks.

Notes:

- TemplateResponse and SimpleTemplateResponse reside in
django.template.response
- django/shortcuts/__init__.py used to have extra spaces. They are
removed.
- *args and **kwargs in TemplateResponse and SimpleTemplateResponse
are gone in order to provide more help for IDEs
- 'content' argument is not allowed for TemplateResponse and
SimpleTemplateResponse now
- context can be omitted now, the only required argument for
SimpleTemplateResponse is the template; request and template are
required for TemplateResponse
- 'response_class' is not implemented but the status code can be
passed as 'status' parameter.
- _set_content doesn't return a value now

The result:

from django.shortcuts import render

def my_view(request):
render(request, 'foo.json', {'foo': 'bar'}, 'application/json',
504)

def extended_view(request)
response = my_view(request)
response.template_context.update({'foo': 'spam', 'baz': 'egg'})
response.template_name = 'spam.json'
response.status_code = 400
return response



On 21 окт, 23:56, Mikhail Korobov  wrote:
> I love programming: two-liner shortcut turns to be a massive core
> refactoring ;) Ivan, thank you for the research.
>
> I'll provide a draft patch for 'render == TemplateResponse' soon.
>
> By the way, Łukasz Rekucki's suggestion to add the 'response_class' to
> render shortcut is complicated much by TemplateResponse because
> SimpleTemplateResponse is inherited from HttpResponse.
>
> On 21 окт, 22:34, Ivan Sagalaev  wrote:
>
>
>
> > On 10/21/2010 03:22 PM, Ivan Sagalaev wrote:
>
> > > On 10/21/2010 11:49 AM, Mikhail Korobov wrote:
> > >> 2. Does TemplateResponse allow pretty exception pages or not? Is Ben's
> > >> issue resolved?
>
> > > I'll look into it this evening (MSD).
>
> > So I did.
>
> > There are actually two problems:
>
> > - Exceptions in response middleware are indeed happen outside of the
> > request's `try .. except` block. This is a problem by itself[1] and I'd
> > be happy to fix it after ticket 9886[2] is committed. It's another
> > refactoring of core request code so I don't want to mess things up doing
> > one patch on top of another :-).
>
> > - Non-pretty plain text tracebacks can be caused not only be middleware
> > but also by any error occurring during template rendering. Because all
> > this happen *after* request was returned to the web server handler and
> > is being iterated over.
>
> > This second problem can be easily fixed by introducing a method for
> > explicit evaluation of the content: `evaluate()` or `force_content()`
> > that will be a noop for any HttpRespone class except the
> > TemplateResponse. The method will be called by the request handler right
> > before returning the response.
>
> > Sounds good?
>
> > P.S. BTW looking at the TemplateResponse implementation I see that Simon
> > had actually intended it to be effectively *the* render shortcut[3]. I
> > find it quite beautiful really :-)
>
> > [1]:http://code.djangoproject.com/ticket/14523
> > [2]:http://code.djangoproject.com/ticket/9886
> > [3]:http://github.com/simonw/django-openid/blob/master/django_openid/resp...

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Ivan Sagalaev

On 10/21/2010 03:22 PM, Ivan Sagalaev wrote:

On 10/21/2010 11:49 AM, Mikhail Korobov wrote:

2. Does TemplateResponse allow pretty exception pages or not? Is Ben's
issue resolved?


I'll look into it this evening (MSD).


So I did.

There are actually two problems:

- Exceptions in response middleware are indeed happen outside of the 
request's `try .. except` block. This is a problem by itself[1] and I'd 
be happy to fix it after ticket 9886[2] is committed. It's another 
refactoring of core request code so I don't want to mess things up doing 
one patch on top of another :-).


- Non-pretty plain text tracebacks can be caused not only be middleware 
but also by any error occurring during template rendering. Because all 
this happen *after* request was returned to the web server handler and 
is being iterated over.


This second problem can be easily fixed by introducing a method for 
explicit evaluation of the content: `evaluate()` or `force_content()` 
that will be a noop for any HttpRespone class except the 
TemplateResponse. The method will be called by the request handler right 
before returning the response.


Sounds good?

P.S. BTW looking at the TemplateResponse implementation I see that Simon 
had actually intended it to be effectively *the* render shortcut[3]. I 
find it quite beautiful really :-)


[1]: http://code.djangoproject.com/ticket/14523
[2]: http://code.djangoproject.com/ticket/9886
[3]: 
http://github.com/simonw/django-openid/blob/master/django_openid/response.py#L85


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Ivan Sagalaev

On 10/21/2010 11:49 AM, Mikhail Korobov wrote:

2. Does TemplateResponse allow pretty exception pages or not? Is Ben's
issue resolved?


I'll look into it this evening (MSD).

--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Mikhail Korobov
Well, I don't mean that now we all must write only class-based views.
I was talking about reusable views and most views don't have to be
reusable (though it would be nice). Django now can help developer in
writing reusable views and it was not the case when TemplateResponse
was invented. That's what I mean with "TemplateResponse is less useful
now". TemplateResponse and class-based views are targeting the same
problem and they are different ways to solve it. I don't know what is
better, the argument was that we already have one way to reuse view
logic.

My statement about "only complicate things in django 1.3" was
premature, sorry. TemplateResponse is indeed a nice way to reuse view
logic that don't require writing a class-based view.

The ticket in DDN is http://code.djangoproject.com/ticket/12815.

So now the questions are:

1. Do we need 2 nice ways for reusing view logic (with and without
classes)? I don't have an opinion on this.
2. Does TemplateResponse allow pretty exception pages or not? Is Ben's
issue resolved?


On 21 окт, 13:25, Ivan Sagalaev  wrote:
> On 10/21/2010 11:10 AM, James Bennett wrote:
>
>
>
>
>
> > Django cares about whether your views meet the following criteria:
>
> > 1. Is a callable object.
> > 2. When called, accepts an instance of HttpRequest as its first
> > positional argument.
> > 3. When called, returns an instance of HttpResponse or raises an exception.
>
> > That's it. Write them as functions, if that makes sense for your use
> > case. Write them as classes with callable instances, if that makes
> > sense for your use case. Generic views are now class-based because
> > that's what seems to work best *for the case of generic views*;
> > subclassing and overriding is, for the type of flexibility the generic
> > views need, simpler and cleaner than supporting long lists of keyword
> > arguments. But the generic views are not all views, and not all views
> > have to do what the generic views do. So my position is that you
> > should think about what you need from your view, and choose
> > class-based or function-based as appropriate.
>
> Thanks, that what I thought.
>
> Going back to the argument about TemplateResponse I can say that it's a
> good way for a view-as-function to keep its response hackable.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread Ivan Sagalaev

On 10/21/2010 11:10 AM, James Bennett wrote:

Django cares about whether your views meet the following criteria:

1. Is a callable object.
2. When called, accepts an instance of HttpRequest as its first
positional argument.
3. When called, returns an instance of HttpResponse or raises an exception.

That's it. Write them as functions, if that makes sense for your use
case. Write them as classes with callable instances, if that makes
sense for your use case. Generic views are now class-based because
that's what seems to work best *for the case of generic views*;
subclassing and overriding is, for the type of flexibility the generic
views need, simpler and cleaner than supporting long lists of keyword
arguments. But the generic views are not all views, and not all views
have to do what the generic views do. So my position is that you
should think about what you need from your view, and choose
class-based or function-based as appropriate.


Thanks, that what I thought.

Going back to the argument about TemplateResponse I can say that it's a 
good way for a view-as-function to keep its response hackable.


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-21 Thread James Bennett
On Thu, Oct 21, 2010 at 1:29 AM, Ivan Sagalaev
 wrote:
> Can someone of core committers clarify this: is it now recommended to write
> all views as classes?

Django cares about whether your views meet the following criteria:

1. Is a callable object.
2. When called, accepts an instance of HttpRequest as its first
positional argument.
3. When called, returns an instance of HttpResponse or raises an exception.

That's it. Write them as functions, if that makes sense for your use
case. Write them as classes with callable instances, if that makes
sense for your use case. Generic views are now class-based because
that's what seems to work best *for the case of generic views*;
subclassing and overriding is, for the type of flexibility the generic
views need, simpler and cleaner than supporting long lists of keyword
arguments. But the generic views are not all views, and not all views
have to do what the generic views do. So my position is that you
should think about what you need from your view, and choose
class-based or function-based as appropriate.


-- 
"Bureaucrat Conrad, you are technically correct -- the best kind of correct."

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Mikhail Korobov
Hi Ivan!

On 21 окт, 03:00, Ivan Sagalaev  wrote:
>
> Wait!!!
>
> Sorry… Hello everyone :-)
>
> If I remember correctly TemplateResponse was solving a problem of some
> middleware wanting to mess with a view context before it's baked into
> final string representation. This would solve in a good way what now is
> accomplished by things like @render_to decorator.
>
> What I don't understand from your reasoning is how class-based views are
> going to help here? From what I see only Django-supplied generic views
> are now class-based and we didn't deprecate simple user view functions.
> Which means that "render" won't be as useful for them if it wouldn't
> provide any means to post-process the context and if the context won't
> be aware of request: these are two main points why people are not happy
> with render_to_response right now.
>

I see this as several separate specific problems / use cases.

1. Developer writes a view and want to reuse it (e.g. change the
context).

My assumptions was:
a) Class based views are now the recommended way to write reusable
views
b) The main benefit from TemplateResponse is the ability to reuse view
responses. I made this assumption because of the example in #12815,
the 'this pattern would be particularly valuable for customising the
admin' statement and the original example in your proposal (
http://groups.google.com/group/django-developers/msg/d5df254f01800ee2
).
c) b) can now be achieved by writing a class-based view

2. Developer wants to write a middleware that messes with view
context. Class-based views are not going to help here. But I thought
it is not nearly as demanded as the first use case. This is useful but
not mandatory. That's where my reasoning came from.

> Mikhail, do you have any actual objections against TemplateResponse or
> you just don't want to complicate your implementation? If it is the
> latter then TemplateResponse has been already implemented[1] by Simon
> Willison. You might just use it.
>
> [1]:http://github.com/simonw/django-openid/blob/master/django_openid/resp...

Thanks for pointing out the implemented TemplateResponse. No, I
haven't actual objections against it and just don't want to complicate
the implementation. There was an issue with TemplateResponse approach
for Ben Firshman ( 
http://groups.google.com/group/django-developers/msg/fc9e0f8810d3e784
) and the ticket is in DDN so it is not as simple as just replace
HttpResponse with TemplateResponse.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Ivan Sagalaev

On 10/20/2010 11:51 PM, Mikhail Korobov wrote:

#12815 is about introducing TemplateResponse. Is the patch with
'render' shortcut returning just HttpResponse acceptable? I think that
TemplateResponse is less useful after class-based views make their way
into trunk so 'render' shortcut shouldn't bother returning
TemplateResponse. There are ways to reuse view logic (and change view
context in particular) now and TemplateResponse (which was a good
addition to django 1.2/1.1/1.0) seems to only complicate things in
django 1.3.


Wait!!!

Sorry… Hello everyone :-)

If I remember correctly TemplateResponse was solving a problem of some 
middleware wanting to mess with a view context before it's baked into 
final string representation. This would solve in a good way what now is 
accomplished by things like @render_to decorator.


What I don't understand from your reasoning is how class-based views are 
going to help here? From what I see only Django-supplied generic views 
are now class-based and we didn't deprecate simple user view functions. 
Which means that "render" won't be as useful for them if it wouldn't 
provide any means to post-process the context and if the context won't 
be aware of request: these are two main points why people are not happy 
with render_to_response right now.


Mikhail, do you have any actual objections against TemplateResponse or 
you just don't want to complicate your implementation? If it is the 
latter then TemplateResponse has been already implemented[1] by Simon 
Willison. You might just use it.


[1]: 
http://github.com/simonw/django-openid/blob/master/django_openid/response.py


--
You received this message because you are subscribed to the Google Groups "Django 
developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Mikhail Korobov
I think the correct ticket is http://code.djangoproject.com/ticket/9081
and it is in 'almost-wontfix' state now. Yes, it's a great time to
either move it to wontfix or mark as accepted and implement alongside
with the render shortcut.

On 21 окт, 02:05, Łukasz Rekucki  wrote:
> On 20 October 2010 21:57, Jacob Kaplan-Moss  wrote:
>
>
>
>
>
> > 2010/10/20 Mikhail Korobov :
> >> There is an unresolved question in the ticket: "The only hesitation is
> >> the relationship with #12815; we should resolve that decision before
> >> committing anything for this ticket."
>
> >> #12815 is about introducing TemplateResponse. Is the patch with
> >> 'render' shortcut returning just HttpResponse acceptable? I think that
> >> TemplateResponse is less useful after class-based views make their way
> >> into trunk so 'render' shortcut shouldn't bother returning
> >> TemplateResponse. There are ways to reuse view logic (and change view
> >> context in particular) now and TemplateResponse (which was a good
> >> addition to django 1.2/1.1/1.0) seems to only complicate things in
> >> django 1.3.
>
> > I agree completely with this reasoning - just render returning an
> > HttpResponse is fine, I think.
>
> Both render_to_response() and direct_to_template() have one very
> annoying flaw:http://code.djangoproject.com/ticket/12669. Please add
> a "response_class" keyword to your render() function ;). Thanks!
>
> --
> Łukasz Rekucki

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Łukasz Rekucki
On 20 October 2010 21:57, Jacob Kaplan-Moss  wrote:
> 2010/10/20 Mikhail Korobov :
>> There is an unresolved question in the ticket: "The only hesitation is
>> the relationship with #12815; we should resolve that decision before
>> committing anything for this ticket."
>>
>> #12815 is about introducing TemplateResponse. Is the patch with
>> 'render' shortcut returning just HttpResponse acceptable? I think that
>> TemplateResponse is less useful after class-based views make their way
>> into trunk so 'render' shortcut shouldn't bother returning
>> TemplateResponse. There are ways to reuse view logic (and change view
>> context in particular) now and TemplateResponse (which was a good
>> addition to django 1.2/1.1/1.0) seems to only complicate things in
>> django 1.3.
>
> I agree completely with this reasoning - just render returning an
> HttpResponse is fine, I think.

Both render_to_response() and direct_to_template() have one very
annoying flaw: http://code.djangoproject.com/ticket/12669. Please add
a "response_class" keyword to your render() function ;). Thanks!

-- 
Łukasz Rekucki

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Jacob Kaplan-Moss
2010/10/20 Mikhail Korobov :
> There is an unresolved question in the ticket: "The only hesitation is
> the relationship with #12815; we should resolve that decision before
> committing anything for this ticket."
>
> #12815 is about introducing TemplateResponse. Is the patch with
> 'render' shortcut returning just HttpResponse acceptable? I think that
> TemplateResponse is less useful after class-based views make their way
> into trunk so 'render' shortcut shouldn't bother returning
> TemplateResponse. There are ways to reuse view logic (and change view
> context in particular) now and TemplateResponse (which was a good
> addition to django 1.2/1.1/1.0) seems to only complicate things in
> django 1.3.

I agree completely with this reasoning - just render returning an
HttpResponse is fine, I think.

Jacob

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Mikhail Korobov
That's great! I'll mark the ticket as assigned for me then.

There is an unresolved question in the ticket: "The only hesitation is
the relationship with #12815; we should resolve that decision before
committing anything for this ticket."

#12815 is about introducing TemplateResponse. Is the patch with
'render' shortcut returning just HttpResponse acceptable? I think that
TemplateResponse is less useful after class-based views make their way
into trunk so 'render' shortcut shouldn't bother returning
TemplateResponse. There are ways to reuse view logic (and change view
context in particular) now and TemplateResponse (which was a good
addition to django 1.2/1.1/1.0) seems to only complicate things in
django 1.3.

On 21 окт, 01:02, Jacob Kaplan-Moss  wrote:
> On Wed, Oct 20, 2010 at 1:48 PM, Mikhail Korobov  
> wrote:
> > So please add the 'render' shortcut in 1.3.
>
> It's one of the things on my list. If you'd like to make it happen
> faster, a patch + tests would make it a no-brainer for me.
>
> Jacob

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.



Re: Gentle Proposal: add 'render' shortcut in 1.3

2010-10-20 Thread Jacob Kaplan-Moss
On Wed, Oct 20, 2010 at 1:48 PM, Mikhail Korobov  wrote:
> So please add the 'render' shortcut in 1.3.

It's one of the things on my list. If you'd like to make it happen
faster, a patch + tests would make it a no-brainer for me.

Jacob

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-develop...@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.