Opened 6 years ago

Last modified 3 months ago

#13910 assigned New feature

Add generator version of Template.render(Context)

Reported by: Ron Panduwana Owned by: Gagaro
Component: Template system Version: master
Severity: Normal Keywords:
Cc: mindsocket, Selwin Ong, Gagaro, binary@… 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 6 years ago.
diff with django-1.2.1 source code
sample-project.zip (3.9 KB) - added by Ron Panduwana 6 years ago.

Download all attachments as: .zip

Change History (35)

Changed 6 years ago by Ron Panduwana

Attachment: django-1.2.1.diff added

diff with django-1.2.1 source code

Changed 6 years ago by Ron Panduwana

Attachment: sample-project.zip added

comment:1 Changed 6 years ago by Russell Keith-Magee

Resolution: duplicate
Status: newclosed

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

comment:2 Changed 6 years ago by Ron Panduwana

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 Changed 6 years ago by Ron Panduwana

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 Changed 6 years ago by Russell Keith-Magee

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 Changed 6 years ago by EmilStenstrom

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 Changed 6 years ago by dmoisset

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 Changed 6 years ago by Graham King

Severity: Normal
Type: New feature

comment:8 Changed 6 years ago by Julien Phalip

Needs tests: set

comment:9 Changed 5 years ago by chase@…

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 Changed 4 years ago by mindsocket

Cc: mindsocket added

comment:11 Changed 4 years ago by Aymeric Augustin

Status: reopenednew

comment:12 Changed 4 years ago by mindsocket

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 Changed 4 years ago by mindsocket

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 Changed 4 years ago by Simon Charette

1.5 introduced StreamingHttpResponse.

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

comment:15 Changed 4 years ago by mindsocket

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 Changed 4 years ago by mindsocket

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 Changed 4 years ago by mindsocket

Needs tests: unset
Version: 1.2master

comment:18 Changed 4 years ago by Selwin Ong

Cc: Selwin Ong added

comment:19 Changed 4 years ago by mindsocket

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 Changed 4 years ago by mindsocket

Owner: changed from nobody to mindsocket
Status: newassigned

comment:21 Changed 3 years ago by anonymous

I love this feature. What's the status?

comment:22 in reply to:  21 Changed 3 years ago by Baptiste Mispelon

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 Changed 2 years ago by Tim Graham

Patch needs improvement: set

comment:24 Changed 18 months ago by Gagaro

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

comment:26 Changed 18 months ago by Gagaro

Cc: Gagaro added

comment:27 Changed 18 months ago by Gagaro

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

comment:28 Changed 17 months ago by Tim Graham

Patch needs improvement: set

comment:29 Changed 17 months ago by Gagaro

Patch needs improvement: unset

comment:30 Changed 16 months ago by Tim Graham

Patch needs improvement: set

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

comment:31 Changed 14 months ago by Tim Graham

Patch needs improvement: unset

Patch has been updated to address the API concerns.

comment:32 Changed 14 months ago by Tim Graham

Patch needs improvement: set

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

comment:33 Changed 3 months ago by Sergey Dobrov

Cc: binary@… added
Note: See TracTickets for help on using tickets.
Back to Top