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)
Change History (39)
by , 14 years ago
Attachment: | django-1.2.1.diff added |
---|
by , 14 years ago
Attachment: | sample-project.zip added |
---|
comment:1 by , 14 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
Duplicate of #7581, plus some work on the wsgi-improvements GSoC branch.
comment:2 by , 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 , 14 years ago
Resolution: | duplicate |
---|---|
Status: | closed → reopened |
Summary: | Add stream_to_response() shortcut that returns streaming HttpResponse → Add 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 , 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 , 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 , 14 years ago
Triage Stage: | Unreviewed → Accepted |
---|
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 , 14 years ago
Severity: | → Normal |
---|---|
Type: | → New feature |
comment:8 by , 14 years ago
Needs tests: | set |
---|
comment:9 by , 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 , 12 years ago
Cc: | added |
---|
comment:11 by , 12 years ago
Status: | reopened → new |
---|
comment:12 by , 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 , 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 , 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 , 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 , 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 , 12 years ago
Needs tests: | unset |
---|---|
Version: | 1.2 → master |
comment:18 by , 12 years ago
Cc: | added |
---|
comment:19 by , 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 , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:22 by , 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 , 11 years ago
Patch needs improvement: | set |
---|
comment:24 by , 10 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Owner: | changed from | to
comment:26 by , 10 years ago
Cc: | added |
---|
comment:27 by , 9 years ago
Needs documentation: | unset |
---|---|
Needs tests: | unset |
Patch needs improvement: | unset |
comment:28 by , 9 years ago
Patch needs improvement: | set |
---|
comment:29 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:30 by , 9 years ago
Patch needs improvement: | set |
---|
Aymeric raised some questions/concerns about the proposed APIs on django-developers.
comment:31 by , 9 years ago
Patch needs improvement: | unset |
---|
Patch has been updated to address the API concerns.
comment:32 by , 9 years ago
Patch needs improvement: | set |
---|
Marking as "Patch needs improvement" given the latest comment about a regression in performance.
comment:33 by , 8 years ago
Cc: | added |
---|
comment:34 by , 6 years ago
Patch needs improvement: | unset |
---|
comment:35 by , 6 years ago
Cc: | added |
---|
comment:37 by , 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
diff with django-1.2.1 source code