Opened 14 years ago

Last modified 5 years ago

#13910 assigned New feature

Add generator version of Template.render(Context)

Reported by: Ron Panduwana Owned by: Petr Glotov
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: mindsocket, Selwin Ong, Gagaro, binary@…, Petr Glotov Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

We already can make a streaming HttpResponse by passing a generator to it. This patch adds a "stream()" method to the Template class that returns a string generator that renders the template node-by-node suitable for passing to such HttpResponse. This also includes a shortcut function "stream_to_response()" -- the streaming version of render_to_response().

Attachments (2)

django-1.2.1.diff (20.3 KB ) - added by Ron Panduwana 14 years ago.
diff with django-1.2.1 source code
sample-project.zip (3.9 KB ) - added by Ron Panduwana 14 years ago.

Download all attachments as: .zip

Change History (39)

by Ron Panduwana, 14 years ago

Attachment: django-1.2.1.diff added

diff with django-1.2.1 source code

by Ron Panduwana, 14 years ago

Attachment: sample-project.zip added

comment:1 by Russell Keith-Magee, 14 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #7581, plus some work on the wsgi-improvements GSoC branch.

comment:2 by Ron Panduwana, 14 years ago

OK, so probably I didn't make myself clear. What I propose is an alternative to Template.render(Context), that instead of doing complete rendering, returns a string generator instead. When consumed, the generator will yield rendered string in a node-by-node, bit-by-bit fashion, and thus suitable for passing to HttpResponse to make a streaming page. I don't change the HttpResponse itself, I change the Template class so it has a method that returns string generator and several built-in template tags so they support rendering their contents in bit-by-bit fashion. And I don't see that #7581/2009-GSoC-wsgi-improvents incorporate such change or something equivalent.

Pardon me if I missed something.

comment:3 by Ron Panduwana, 14 years ago

Resolution: duplicate
Status: closedreopened
Summary: Add stream_to_response() shortcut that returns streaming HttpResponseAdd generator version of Template.render(Context)

For example, this is an example I use on the patch's documentation:

Suppose we have a template like this:

    {% for item in warehouse %}
        Current value: {{ item.calculate_depreciation }}
    {% endfor %}

If there are 100 items in warehouse, and calculate_depreciation() takes half seconds, Template.render(Context) will take 50 seconds to complete.

The patch adds Template.stream(Context) which returns string generator instead, which can be given to HttpResponse, so we can yield one item per 0.5 seconds instead of making the browser wait until all items calculated.

comment:4 by Russell Keith-Magee, 14 years ago

My apologies - this is a separate issue, albeit one that is closely related to #7581. Specifically, there's no point having a generator/iterable template object if you have middlewares that require consumption of the entire generator before they will produce output. Ensuring that HttpResponse can fully handle generator content is essentially a prerequisite for this change.

comment:5 by EmilStenstrom, 14 years ago

Some context to why I think this is important:

For performance reasons it's good to have your static media files accessed as fast as possible. In PHP you can achieve this by calling flush() directly after the </head> tag. This make sure the media files start loading as fast as possible, even though the rest of the page is still doing expensive db operations. This is also mentioned in Yahoo's guidelines for performance: http://developer.yahoo.com/performance/rules.html#flush

comment:6 by Daniel F Moisset, 14 years ago

Triage Stage: UnreviewedAccepted

Flagging this as Accepted, even if it depends on #7581 which is DDN. No matter what technical decision is taken in #7581, nobody is talking there of removing streaming responses (which have been supported and documented since at least 1.0), so having the possibility of rendering a template as an iterator looks like a perfectly acceptable proposal/issue. And useful to people aware of #7581 (as anyone using iterators in resposnses today should be)

comment:7 by Graham King, 14 years ago

Severity: Normal
Type: New feature

comment:8 by Julien Phalip, 14 years ago

Needs tests: set

comment:9 by chase@…, 14 years ago

Easy pickings: unset
UI/UX: unset

Being able to stream a template alone would be useful, even without being able to stream a HTTP response. I'm thinking of using a template to generate a large amount of data (XML?), and wanting to stream that to disk versus read it all into memory first.

comment:10 by mindsocket, 12 years ago

Cc: mindsocket added

comment:11 by Aymeric Augustin, 12 years ago

Status: reopenednew

comment:12 by mindsocket, 12 years ago

I'm hoping to revisit this ticket, are there any updates or others working on it already? If not, any changes since 1.5 that might influence how it should be implemented?

comment:13 by mindsocket, 12 years ago

I've put my work in progress in a branch: https://github.com/mindsocket/django/tree/t13910

So far I've taken rooney's patch and updated it to work with current development. The template_tests pass, but some others are failing, and new tests are needed for the new streaming code.

comment:14 by Simon Charette, 12 years ago

1.5 introduced StreamingHttpResponse.

There's already a TemplateResponse. Maybe you can start your work by providing a StreamingTemplateResponse?

comment:15 by mindsocket, 12 years ago

Thanks charettes. I've updated my branch to improve on what I had (including correct use of a StreamingHttpResponse and new unit tests), but I will have a look at TemplateResponse now too.

comment:16 by mindsocket, 12 years ago

I've now added a StreamingTemplateResponse and StreamingTemplateView that build upon the earlier work to introduce generator based rendering of templates.

I've tried to include new tests where possible, but feedback about where they're lacking is most welcome.

https://github.com/mindsocket/django/tree/t13910

Some background to explain my interest in this ticket, I created a sample app that makes use of the template streaming in this branch.
It's an experimental proof of concept that uses a middleware to start sending back HTML before even calling the view.

Code: https://github.com/mindsocket/django-perf-example/blob/master/perf_example/views.py
Results: http://www.webpagetest.org/video/compare.php?tests=130428_WX_F4V-l:eager_streaming%2C130428_W6_F42-l:streaming%2C130428_3X_F4E-l:original&thumbSize=200&ival=500&end=doc

comment:17 by mindsocket, 12 years ago

Needs tests: unset
Version: 1.2master

comment:18 by Selwin Ong, 12 years ago

Cc: Selwin Ong added

comment:19 by mindsocket, 12 years ago

Having given this patch more of a workout I've since fixed a test bug and created a pull request...

https://github.com/django/django/pull/1037

My only concern about the commit is whether more documentation updates are needed for aspects higher up the stack, like StreamingTemplateView. I'm not entirely sure how I'd go about explaining the benefits, specific use cases and most importantly downsides (like exception handling) of streaming without lots of hand waving.

comment:20 by mindsocket, 12 years ago

Owner: changed from nobody to mindsocket
Status: newassigned

comment:21 by anonymous, 11 years ago

I love this feature. What's the status?

in reply to:  21 comment:22 by Baptiste Mispelon, 11 years ago

Replying to anonymous:

I love this feature. What's the status?

For one thing, the provided pull request no longer applies cleanly on master so it needs to be brought back up to date.

comment:23 by Tim Graham, 11 years ago

Patch needs improvement: set

comment:24 by Gagaro, 10 years ago

Needs documentation: set
Needs tests: set
Owner: changed from mindsocket to Gagaro

comment:26 by Gagaro, 10 years ago

Cc: Gagaro added

comment:27 by Gagaro, 9 years ago

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:28 by Tim Graham, 9 years ago

Patch needs improvement: set

comment:29 by Gagaro, 9 years ago

Patch needs improvement: unset

comment:30 by Tim Graham, 9 years ago

Patch needs improvement: set

Aymeric raised some questions/concerns about the proposed APIs on django-developers.

comment:31 by Tim Graham, 9 years ago

Patch needs improvement: unset

Patch has been updated to address the API concerns.

comment:32 by Tim Graham, 9 years ago

Patch needs improvement: set

Marking as "Patch needs improvement" given the latest comment about a regression in performance.

comment:33 by Sergey Dobrov, 8 years ago

Cc: binary@… added

comment:34 by Asif Saifuddin Auvi, 6 years ago

Patch needs improvement: unset

comment:35 by Petr Glotov, 6 years ago

Cc: Petr Glotov added

comment:36 by Carlton Gibson, 5 years ago

Owner: changed from Gagaro to Petr Glotov

comment:37 by Carlton Gibson, 5 years ago

Patch needs improvement: set

Patch includes changes to shortcuts, generic views and so-on, which are of questionable value, and out-of-scope for this ticket. Needs stripping down to just allowing a render() to return a generator, but should be good. Comments on PR

Note: See TracTickets for help on using tickets.
Back to Top