[Django] #25839: RequestContext does not apply context processors [regression]
Django
---------------------------------+----------------------------------------
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Severity: Release blocker | Keywords: RequestContext, regression
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Prior to Django 1.8 `django.template.RequestContext` always had the
context from all of the `TEMPLATE_CONTEXT_PROCESSORS` applied. In Django
1.8 this is no longer the case, which breaks the API. As this has neither
been announced nor documented anywhere I'd consider this a critical
regression.
The following view code no longer works with the default
`TEMPLATE_CONTEXT_PROCESSORS` setting:
{{{
from django.template import RequestContext
def my_view(request):
context = RequestContext(request)
print context['user']
#...
}}}
Actually `user` should be put there by
`django.contrib.auth.context_processors.auth`. It is also present in
templates rendered by `render()`, but not in `RequestContext`. Of course
this does not only apply to the `user` context variable, but to all
context variables set by context processors.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* cc: aaugustin (added)
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Comment:
Bisected to 37505b6397058bcc3460f23d48a7de9641cd6ef0. The test I used to
bisect is attached.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:1>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* Attachment "test_25839.py" added.
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
As part of the multiple-template-engines feature, Django 1.8 refactored
the Django template language to remove the direct coupling to global
settings, instead making it possible to instantiate multiple
independently-configured `Engine` instances (and configure multiple such
instances via the new `TEMPLATES` setting). Because of this, it's no
longer possible for `RequestContext` to automatically fetch the list of
configured context processors from settings (the `TEMPLATES` setting can
define multiple instances of the DTL with different lists of context
processors, so how could `RequestContext` possibly know which list of
context processors to use?). This was handled by adding a new context-
manager method `RequestContext.bind_template` which temporarily binds a
`RequestContext` to a template instance, in the process applying all the
context processors from that template's engine configuration.
This solution is fully backwards-compatible for the only documented usage
of a `RequestContext`, which is to pass it into a template render and
access its values in the template. It's true that it's not backwards-
compatible for the usage you demonstrate, directly accessing values from
the context as a dictionary without rendering a template, but this is not
a documented or supported mode of use for `RequestContext`. If you need
this usage, you must now first bind the context to a specific template:
{{{
from django.template import engines
engine = engines['default']
tpl = engine.get_template('some/template.html')
ctx = RequestContext(request)
with ctx.bind_template(tpl):
print(ctx['user'])
}}}
It may be possible to restore the old behavior for this usage using
`Engine.get_default()` (it would only work if there's only a single
instance of the DTL configured in settings); personally I don't think
that's worth doing, or that this use of `RequestContext` should be
supported.
Alternatively, we could add a warning to the 1.8 release notes about this,
but this usage is so unusual that I'm not even sure that's worth doing.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:2>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* severity: Release blocker => Normal
Comment:
Thanks Carl, you've explained the situation more clearly than I would have
been able to. This is the intended behavior. It does '''not''' break the
'''public''' API.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:3>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by direx):
By saying that this is not part of the public API or not a supported usage
of `RequestContext` you are making this a little too easy for you.
After looking at the Django 1.7 documentation these are facts:
1. `RequestContext` is a special `Context` class. One of the differences
„''is that it automatically populates the context with a few variables,
according to your `TEMPLATE_CONTEXT_PROCESSORS` setting.''“ (quote from
documentation)
2. Another quote from the documentation: „''Most of the time, you’ll
instantiate Context objects by passing in a fully-populated dictionary to
Context(). But you can add and delete items from a Context object once
it’s been instantiated, too, using standard dictionary syntax''“. Note the
'''standard dictionary syntax''' here.
So I don't see why the behavior described in this bug should not be
considered part of the public API. The ''usage as dictionary'' is actually
even documented and encouraged (also in 1.8, section ''Playing with
Context objects''). So I'd still consider this a public API breakage.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:4>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
Can you submit a patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:5>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Component: Template system | Version: 1.8
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
(In case that's not clear enough, saying "you are making this a little too
easy for you" to a person who works in their free time guarantees their
non-cooperation. My involvement with this issue stops here.)
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:6>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* component: Template system => Documentation
Comment:
That's a good point; I looked through the `RequestContext` docs but didn't
notice that dictionary-style access was documented higher up in the
`Context` docs. I think that does make it a backwards-incompatible change
to a public API that should have been documented in the release notes.
However, I still don't think it's a valuable enough usage to fix at this
point. If we had run across this case while building multiple-template-
engines, I think we'd have documented it as a minor backwards-
incompatibility, not attempted to accommodate it -- the usage just doesn't
belong in a multiple-template-engines world.
I'd be willing to look at a proposed patch, but I think a patch will need
to introduce too much complexity. Specifically, a patch to reintroduce the
old behavior would need to introduce a scheme for lazy-loading of the
"default" context processors (that is, the ones configured for the
"default" DTL instance -- which doesn't exist if there's more than one DTL
instance configured) that takes place only if someone tries to dictionary-
access a RequestContext that hasn't been bound to a template. This would
be an entirely new code path that exists only to support this usage. I
don't think that added complexity is justified by the limited-to-
nonexistent need for this usage (note that the docs don't ever recommend
it for any real purpose, just for "playing around" to understand Context
objects).
So I think this ticket should be fixed the same way we'd have fixed it if
we'd noticed it before the release of 1.8: by making a note of the issue
in the backwards-incompatible changes section of the 1.8 release notes,
and perhaps also clarifying the RequestContext docs to note that values
from context processors are only available in a RequestContext once it has
been bound to a template (which Django does automatically when you render
a template using a RequestContext).
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:7>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by direx):
Replying to [comment:6 aaugustin]:
> (In case that's not clear enough, saying "you are making this a little
too easy for you" to a person who works in their free time guarantees
their non-cooperation. My involvement with this issue stops here.)
I don't get your point. I was neither rude, nor was my comment meant to be
offensive. I am also reporting bugs in my free time, hoping to help other
users who might stumble upon the same issue. Sorry if you got me wrong
there.
Replying to [comment:7 carljm]:
> So I think this ticket should be fixed the same way we'd have fixed it
if we'd noticed it before the release of 1.8: by making a note of the
issue in the backwards-incompatible changes section of the 1.8 release
notes, and perhaps also clarifying the RequestContext docs to note that
values from context processors are only available in a RequestContext once
it has been bound to a template (which Django does automatically when you
render a template using a RequestContext).
To me this entire issue is actually not a biggie. Some third-party apps I
am using rely on that behavior, but I can fix that. So I'd agree with you
here that adding it to the list of backwards-incompatible changes and
maybe a small note in the docs would probably be sufficient.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:8>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
It took me on the order of magnitude of one week of work to come up with
the patch discussed here in order to maximize backwards compatibility —
most likely because I'm a poor developer. Regardless I didn't try to cut
corners and this was certainly not "easy for me". I'm sorry it didn't work
for you.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:9>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
@direx Thanks for reporting the issue, and following up with your
observations about what behaviors were documented! I'm glad we're agreed
on the best path forward.
FWIW I didn't take the "too easy" comment personally on an individual
level, but rather on the project level, which includes everyone commenting
here. As a project, the initial response here was to disavow any
responsibility at all for this change on a backwards-compatibility front,
and I do indeed (in retrospect) think that was a bit "making it too easy,"
purely in a what-do-we-need-to-do-now-about-this-issue-as-a-project sense.
Fortunately I don't think the documentation-based resolution is "making it
too hard" either :-) I certainly didn't see the comment as a criticism of
the (large amount of excellent) work that went into the initial multiple-
template-engines change; unintentional minor regressions happen, even with
much smaller feature additions than that one.
@direx, I'm curious if you could point me to the third-party app(s) making
use of this behavior? I doubt it will change my mind on what we should do,
but I am curious to see how they found it useful.
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:10>
Django
Reporter: direx | Owner: nobody
Type: Bug | Status: closed
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: wontfix
Keywords: RequestContext, | Triage Stage: Accepted
regression |
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => wontfix
Comment:
As 1.8 is nearly a year old and we only have a single report of the issue,
I say it's not worth documenting unless someone offers a patch. If so,
please reopen. Thanks!
--
Ticket URL: <https://code.djangoproject.com/ticket/25839#comment:11>