Re: Vote on {% include %} behaviour.
I would personally prefer to be able to specify whether the "include" should be rendered at the compile time or at the rendering time. For some applications it is really useful to have recursive inclusion of templates, which is impossible with compile-time inclusion. For example, a few days ago I was writing a view which visualizes a diff between two mongoDB documents with arbitrary levels of nesting. It can be elegantly expressed with recursive includes, yet I was forced to add additional render_to_string statements to my view, unnecessary complicating the code. The problem with processing the inclusion at render time to me is that: x Errors are left unnoticed for a longer time - eg if included template has any errors you won't see them until you are actually processing the view, and if your include is conditional it can get complicated. x Degrades performance. I can't really think of an elegant syntax to that though. {% include "blah.html" render %} seems ugly and would be very confusing to beginners. George On Jun 3, 6:04 pm, Tai Lee wrote: > G'day, > > There are several open tickets (some getting quite old now) that all > stem from the inconsistent behaviour of the {% include %} tag. When a > quoted string is used for the path, it is treated as a special case > and the include is executed at compile time. Otherwise, it is executed > as the template is rendered. > > The docs don't seem to mention this special case, and a few examples > of buggy behaviour and confusion stemming from this include: > > - Included templates with a missing path sometimes fail silently, and > sometimes loudly. > > - Template loaders raise TemplateDoesNotExist when the specified > template *does* exist, if it happens to use an include with a quoted > path that references another template that does not exist. > > - Included templates do not render in the full context of their > parent template as implied by the docs. This means you can't use `{% > block %}` tags inside included templates and override them in > templates that extend the parent template. > > My feeling is that including templates should be consistent. If we > support variables as the path argument to the include tag, then ALL > uses of the include tag should be processed when the templates are > rendered. > > This means that it will not be possible to use block tags inside > includes. This is not possible now, but there is an open ticket that > would like to make it possible (only for the special case quoted > string path includes). > > I would also like to see includes either always fail loudly, or always > fail silently. I think that the most common and expected behaviour > currently is for includes to fail silently, but I don't really care > either way. > > The best solution that I can see is to remove `ConstantIncludeNode` > entirely (as has been suggested in at least one of the tickets). > Includes would always be processed at render time, it would not be > possible to extend blocks defined inside an included template, and > including templates that does not exist would no longer raise a > TemplateDoesNotExist exception when getting the parent template (which > does exist). > > Could we get a quick vote from any core committers and other > interested parties, so that we can move forward to fixing and closing > these tickets? > > https://code.djangoproject.com/ticket/16147https://code.djangoproject.com/ticket/12064https://code.djangoproject.com/ticket/12008https://code.djangoproject.com/ticket/598 > > Cheers. > Tai. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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: Vote on {% include %} behaviour.
#12008 was repurposed as a documentation improvement because the current implementation is correct and just needs to be explained better. As for the other two... I agree that consistency is important, and that it would make sense (in a way) for ConstantIncludeNode to either always raise or always silence, but that can definitely be done without removing the CIN or hacking together some other way to get similar compilation caching. It's also worth mentioning that if TEMPLATE_DEBUG is set to ``True``, the (non-constant) IncludeNode will raise an exception when it *renders*, even though the Django docs clearly state that "render() should never raise TemplateSyntaxError or any other exception. It should fail silently, just as template filters should." If anything, this is a more egregious error than ConstantIncludeNode's behavior. That being said, whatever is decided, perhaps it could be applied to the ExtendsNode as well? Right now, constant extends are loaded at the same time as variable extends - handling constants differently could give a similar performance boost there. In fact, there's a ticket open to do just that: https://code.djangoproject.com/ticket/6586. On Jun 3, 5:12 pm, Luke Plant wrote: > On 03/06/11 17:36, Aymeric Augustin wrote: > > > ConstantIncludeNode improves performance because it calls > > django.template.loader.get_template() only once in __init__() and not > > in each call to render(). > > > get_template() invokes the template loading machinery to create a > > compiled Template object, given a template path. If it's a > > performance bottleneck, we can memoize its results. That will improve > > performance of all template lookup operations, not only {% include > > %}. > > We already have the cached template loader, and we don't want to invoke > that kind of behaviour unless asked for (it's up to users to put it in > their TEMPLATE_LOADERS settings), as it has significant memory overhead. > > I was thinking something simpler, like this: > > class IncludeNode(BaseIncludeNode): > def __init__(self, template_name, *args, **kwargs): > super(IncludeNode, self).__init__(*args, **kwargs) > self.template_name = template_name > > def render(self, context): > try: > template_name = self.template_name.resolve(context) > if getattr(self, 'last_template_name', None) == \ > template_name: > template = self.template > else: > template = get_template(template_name) > self.template = template > self.last_template_name = template_name > return self.render_template(template, context) > except: > if settings.TEMPLATE_DEBUG: > raise > return '' > > If I'm thinking correctly, that will keep the included template in > memory only for the lifetime of the parent template, and will avoid > loading it more than once if it is a static string and the include is in > a loop. > > Luke > > -- > "Underachievement: The tallest blade of grass is the first to be > cut by the lawnmower." (despair.com) > > Luke Plant ||http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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: Vote on {% include %} behaviour.
On 03/06/11 17:36, Aymeric Augustin wrote: > ConstantIncludeNode improves performance because it calls > django.template.loader.get_template() only once in __init__() and not > in each call to render(). > > get_template() invokes the template loading machinery to create a > compiled Template object, given a template path. If it's a > performance bottleneck, we can memoize its results. That will improve > performance of all template lookup operations, not only {% include > %}. We already have the cached template loader, and we don't want to invoke that kind of behaviour unless asked for (it's up to users to put it in their TEMPLATE_LOADERS settings), as it has significant memory overhead. I was thinking something simpler, like this: class IncludeNode(BaseIncludeNode): def __init__(self, template_name, *args, **kwargs): super(IncludeNode, self).__init__(*args, **kwargs) self.template_name = template_name def render(self, context): try: template_name = self.template_name.resolve(context) if getattr(self, 'last_template_name', None) == \ template_name: template = self.template else: template = get_template(template_name) self.template = template self.last_template_name = template_name return self.render_template(template, context) except: if settings.TEMPLATE_DEBUG: raise return '' If I'm thinking correctly, that will keep the included template in memory only for the lifetime of the parent template, and will avoid loading it more than once if it is a static string and the include is in a loop. Luke -- "Underachievement: The tallest blade of grass is the first to be cut by the lawnmower." (despair.com) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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: Vote on {% include %} behaviour.
On 3 juin 2011, at 16:44, Luke Plant wrote: > From what I've seen so far, your proposal sounds good. I agree — consistency is good. > Is there a way > that we can keep the performance benefits of ConstantIncludeNode? For > example, if an IncludeNode detects that it resolves to the same template > path as last time, it doesn't load the template from disk again? If so, > that would remove my only objection. ConstantIncludeNode improves performance because it calls django.template.loader.get_template() only once in __init__() and not in each call to render(). get_template() invokes the template loading machinery to create a compiled Template object, given a template path. If it's a performance bottleneck, we can memoize its results. That will improve performance of all template lookup operations, not only {% include %}. We must ensure that during development, if we edit a template without touching the Python code, we get the new version and not the old compiled, memoized version. The easiest is to memoize only when TEMPLATE_DEBUG is False. There is already some code to cache loaded templates in django.template.loaders.cached.Loader, but as far as I can tell, it's only used during Django's own tests. This is also true for the reset() method of template loaders. If memoizing doesn't work — it's a bit crude — maybe we could extend this code to cache all template loading operations. Best regards, -- Aymeric Augustin. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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: Vote on {% include %} behaviour.
On 03/06/11 09:04, Tai Lee wrote: > Could we get a quick vote from any core committers and other > interested parties, so that we can move forward to fixing and closing > these tickets? >From what I've seen so far, your proposal sounds good. Is there a way that we can keep the performance benefits of ConstantIncludeNode? For example, if an IncludeNode detects that it resolves to the same template path as last time, it doesn't load the template from disk again? If so, that would remove my only objection. Regards, Luke -- "Underachievement: The tallest blade of grass is the first to be cut by the lawnmower." (despair.com) Luke Plant || http://lukeplant.me.uk/ -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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: Vote on {% include %} behaviour.
On Jun 3, 9:32 pm, Jonathan Slenders wrote: > I really never want to have the {% block %} names of B/C in previous > example to be available for overriding in templates which inherit from > A. This would even cause unexpected collisions between block names. > The author of the include B, is not supposed to know where his > template will be included, and we can't expect him to choose block > names which don't collide with those of A. I agree, and my preferred fix (to remove `ConstantIncludeNode`) would make it clear that the actual include is to occur at render time, and any blocks contained therein are completely separate to blocks in the parent template. It should not be possible to replace a block in an included template by defining the same block in the including (parent) template. > Further, I'm not aware of any inconsistencies in include behaviour > when using variable template names. Maybe anyone else? One inconsistency is that when a literal quoted string is used as the path to the template to be included, `ConstantIncludeNode` is used instead of `IncludeNode` as a special case. This tries to include the specified template when the parent template is compiled instead of when it is rendered. This is supposed to be functionally the same as using a variable path argument (which is executed when the template is rendered), but slightly optimised. Unfortunately, it's not functionally the same. One example of this difference causing a problem is that when `TEMPLATE_DEBUG` is `True`, a `TemplateDoesNotExist` exception is raised when compiling a template that does exist, if it contains `{% include "template_that_doesnt_exist.html" %}`. This means that any code that checks for the existence of a template (with `select_template()`) could break and behave differently in production and development environments based on the content of the template. If the template tries to include the same non-existant template, but passed to `{% include %}` as a variable, no exception is raised. Another example cited in the tickets is that it is impossible to conditionally include a template that uses a literal string as the path argument. E.g. `{% if some_false_var %}{% include "..." %}{% endif %}` will raise `TemplateDoesNotExist` because the include is executed when the parent is compiled, even though it is never rendered because the context at render time causes the condition to fail. Cheers. Tai. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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: Vote on {% include %} behaviour.
> This means that it will not be possible to use block tags inside > includes. This is not possible now, but there is an open ticket that > would like to make it possible (only for the special case quoted > string path includes). It's certainly possible to use {% include %} inside a template. It's for the following case, which I often do: > A includes B; B extends from C. I really never want to have the {% block %} names of B/C in previous example to be available for overriding in templates which inherit from A. This would even cause unexpected collisions between block names. The author of the include B, is not supposed to know where his template will be included, and we can't expect him to choose block names which don't collide with those of A. I can understand you want to override some part of the include B in the main template A, that's what I made {% decorate %} for. (Which I still would like to become a django core tag.) It's different from {% include %}. https://github.com/citylive/django-template-tags/blob/master/src/django_template_tags/templatetags/decorate.py Further, I'm not aware of any inconsistencies in include behaviour when using variable template names. Maybe anyone else? Cheers, Jonathan -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.
Vote on {% include %} behaviour.
G'day, There are several open tickets (some getting quite old now) that all stem from the inconsistent behaviour of the {% include %} tag. When a quoted string is used for the path, it is treated as a special case and the include is executed at compile time. Otherwise, it is executed as the template is rendered. The docs don't seem to mention this special case, and a few examples of buggy behaviour and confusion stemming from this include: - Included templates with a missing path sometimes fail silently, and sometimes loudly. - Template loaders raise TemplateDoesNotExist when the specified template *does* exist, if it happens to use an include with a quoted path that references another template that does not exist. - Included templates do not render in the full context of their parent template as implied by the docs. This means you can't use `{% block %}` tags inside included templates and override them in templates that extend the parent template. My feeling is that including templates should be consistent. If we support variables as the path argument to the include tag, then ALL uses of the include tag should be processed when the templates are rendered. This means that it will not be possible to use block tags inside includes. This is not possible now, but there is an open ticket that would like to make it possible (only for the special case quoted string path includes). I would also like to see includes either always fail loudly, or always fail silently. I think that the most common and expected behaviour currently is for includes to fail silently, but I don't really care either way. The best solution that I can see is to remove `ConstantIncludeNode` entirely (as has been suggested in at least one of the tickets). Includes would always be processed at render time, it would not be possible to extend blocks defined inside an included template, and including templates that does not exist would no longer raise a TemplateDoesNotExist exception when getting the parent template (which does exist). Could we get a quick vote from any core committers and other interested parties, so that we can move forward to fixing and closing these tickets? https://code.djangoproject.com/ticket/16147 https://code.djangoproject.com/ticket/12064 https://code.djangoproject.com/ticket/12008 https://code.djangoproject.com/ticket/598 Cheers. Tai. -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@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.