[Django] #16147: {% include %} tag raises TemplateDoesNotExist at compile time of parent template if TEMPLATE_DEBUG is True
Django
template if TEMPLATE_DEBUG is True
-------------------------------------------------+-------------------------
Reporter: mrmachine | Owner: nobody
Type: Bug | Status: new
Milestone: 1.4 | Component:
Version: 1.3 | Template system
Keywords: include template | Severity: Normal
TemplateDoesNotExist TEMPLATE_DEBUG | Triage Stage:
Has patch: 0 | Unreviewed
| Easy pickings: 0
-------------------------------------------------+-------------------------
I think this is a bug. Or at the very least, it can make for unexpected
behaviour that is difficult to trace. When developing locally, `DEBUG` and
`TEMPLATE_DEBUG` are often `True`. When `TEMPLATE_DEBUG` is `True`,
compiling a template that contains an `{% include %}` to a template that
doesn't exist will raise `TemplateDoesNotExist` at compile time of the
parent template.
{{{
>>> from django.template import Template
>>> t = Template('{% include "missing.html" %}')
Traceback (most recent call last):
...
TemplateDoesNotExist: missing.html
}}}
The problem is that when you try to conditionally load a template, and
perform a different action if it does not exist. For example, you might
have a view that uses `select_template` to load one of several templates
(if one exists) and send an email. If no email was sent (because no
template exists), render a different template/context to the response or
take some other action. If the email template contains a typo in the name
of an included template, the email template will not be loaded and no
email will be sent because `TemplateDoesNotExist` was raised.
In the local dev environment, this will not display a pretty debug page
showing the error inside the email template, it will simply behave
unexpectedly (by not sending an email when the email template DOES exist).
On the production server (or when `TEMPLATE_DEBUG` is `False`), it will
behave as expected. The email template will be found and it will be
rendered.
This problem doesn't exist when using a variable as the template name.
{{{
>>> t = Template('{% include somevar %}')
>>>
}}}
Why does Django try to execute the `{% include %}` when it contains a hard
coded string at compile time at all? I think this is a case of premature
optimisation. If it execution of the `{% include %}` was delayed until
render time, `select_template` would correctly return the email template
when `TEMPLATE_DEBUG` is `True`, and the proper error would be displayed
in the rendered email template (making it an easy fix to correct the
typo).
--
Ticket URL: <https://code.djangoproject.com/ticket/16147>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
template if TEMPLATE_DEBUG is True
Type: Bug | Status: new
Version: 1.3 | Severity: Normal
Resolution: | Keywords: include template
Triage Stage: Design | TemplateDoesNotExist TEMPLATE_DEBUG
decision needed | Has patch: 1
Needs documentation: 0 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* needs_docs: => 0
* has_patch: 0 => 1
* stage: Unreviewed => Design decision needed
* needs_tests: => 1
* needs_better_patch: => 1
Comment:
Replying to:
> Why does Django try to execute the {% include %} when it contains a hard
coded string at compile time at all?
[[BR]]
This feature (or bug) has "always" existed. I couldn't find an explanation
in the code or the commit messages.
- It was added at r1349, which introduced the `{% include %}` tag.
- It was moved from django/core/template/loader.py to
django/template/loader_tags.py at r1443, and not changed since then.
Both changes were committed by Adrian in Nov '05.
Hopefully the discussion on the mailing list will help sort it out:
https://groups.google.com/group/django-
developers/browse_thread/thread/f1c2e3664c8aace2
----
Technically, the problem is the implementation of
`django.template.loader_tags.ConstantIncludeNode`: the template resolution
with `get_template` should be moved from `__init__` to `render`.
I'm attaching a patch that fixes it with the minimum amount of changes,
just to show the idea. However:
- after the patch, `ConstantIncludeNode` and `IncludeNode` are very
similar; some refactoring would be useful,
- it breaks the test suite.
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:1>
Django
template if TEMPLATE_DEBUG is True
Type: Bug | Status: new
Version: 1.3 | Severity: Normal
Resolution: | Keywords: include template
Triage Stage: Design | TemplateDoesNotExist TEMPLATE_DEBUG
decision needed | Has patch: 1
Needs documentation: 0 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
The original reporter found the explanation for this optimization and
posted it on IRC:
https://code.djangoproject.com/ticket/598#comment:2
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:2>
Django
template if TEMPLATE_DEBUG is True
Type: Bug | Status: new
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Design
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by prestontimmons):
* needs_better_patch: 1 => 0
* version: 1.3 => 1.4-alpha-1
* needs_tests: 1 => 0
* ui_ux: => 0
Comment:
I added an updated patch with tests.
It's a comprehensive patch that replaces BaseIncludeNode,
ConstantIncludeNode, and IncludeNode with a single implementation.
I also rewrote the tests in a form I find readable, but kept the existing
tests in place so a reviewer can verify backwards compatibility. Besides
that, the old tests are redundant.
In addition to this ticket, this patch addresses #12064 and #3544.
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:4>
Django
template if TEMPLATE_DEBUG is True
Type: Bug | Status: new
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Design
TEMPLATE_DEBUG | decision needed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by prestontimmons):
* cc: prestontimmons@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:5>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Design
TEMPLATE_DEBUG | decision needed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Shortening title because it degrades the layout of Trac :)
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:6>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Design
TEMPLATE_DEBUG | decision needed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: anton@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:7>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Template system | Version:
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Design
TEMPLATE_DEBUG | decision needed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => aaugustin
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:8>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: aaugustin
Type: Bug | Status: new
Component: Template system | Version:
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Design
TEMPLATE_DEBUG | decision needed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by prestontimmons):
Updated the patch and added a pull request for review.
https://github.com/django/django/pull/920
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:9>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: aaugustin
Type: Bug | Status: new
Component: Template system | Version:
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TEMPLATE_DEBUG | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by jacob):
* stage: Design decision needed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:10>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: aaugustin
Type: Bug | Status: new
Component: Template system | Version:
Severity: Normal | 1.4-alpha-1
Keywords: include template | Resolution:
TemplateDoesNotExist | Triage Stage: Accepted
TEMPLATE_DEBUG | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* cc: FunkyBob (added)
Comment:
I've made a PR for a simpler patch for this:
https://github.com/django/django/pull/1528
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:11>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: aaugustin
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: include template | Triage Stage: Ready for
TemplateDoesNotExist | checkin
TEMPLATE_DEBUG | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* version: 1.4-alpha-1 => master
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:12>
Django
TEMPLATE_DEBUG is True
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: aaugustin
Component: Template system | Version: master
Keywords: include template | Triage Stage: Ready for
TemplateDoesNotExist | checkin
TEMPLATE_DEBUG | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"e2f06226ea4a38377cdb69f2f5768e4e00c2d88e"]:
{{{
#!CommitTicketReference repository=""
revision="e2f06226ea4a38377cdb69f2f5768e4e00c2d88e"
Improved {% include %} implementation
Merged BaseIncludeNode, ConstantIncludeNode and Include node.
This avoids raising TemplateDoesNotExist at parsing time, allows recursion
when passing a literal template name, and should make TEMPLATE_DEBUG
behavior
consistant.
Thanks loic84 for help with the tests.
Fixed #3544, fixed #12064, fixed #16147
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16147#comment:13>