Opened 18 months ago
Closed 17 months ago
#34692 closed Bug (fixed)
django.forms.renderers.get_default_renderer()'s template loader cache is not being reset on autoloads.
Reported by: | Andrew | Owned by: | Priyank Panchal |
---|---|---|---|
Component: | Forms | Version: | 4.2 |
Severity: | Normal | Keywords: | |
Cc: | David Smith | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Whenever a template file changes,
django.template.autoreload.template_changed() is called, which in turn calls
django.template.autoreload.reset_loaders(), which clears the template cache in every backend.engine's template loaders.
However, Django's renderer for Forms uses forms.renderers.get_default_renderer(), which returns a mixin that has its own engine instance, and therefore its own loaders (and template caches). This engine is never reset on autoloads. This means that any changes to a template that is referenced as part of, EG "Form.template_name", is never refreshed on changes. The entire runserver process must be restarted for any templates cached through forms.renders to be reloaded. This is tedious and does not match the behavior seen by non-form cached templates.
A crude fix can be demonstrated by changing reset_loaders() to the following:
def reset_loaders(): for backend in engines.all(): if not isinstance(backend, DjangoTemplates): continue for loader in backend.engine.template_loaders: loader.reset() # this code is new: reset the form renderer's template cache as well from django.forms.renderers import get_default_renderer backend = get_default_renderer().engine if isinstance(backend, DjangoTemplates): for loader in backend.engine.template_loaders: loader.reset()
Change History (13)
comment:1 by , 18 months ago
Description: | modified (diff) |
---|---|
Summary: | django.forms.get_default_renderer()'s template loader cache is not being reset on autoloads. → django.forms.renderers.get_default_renderer()'s template loader cache is not being reset on autoloads. |
comment:2 by , 18 months ago
Description: | modified (diff) |
---|
comment:3 by , 18 months ago
Cc: | added |
---|---|
Component: | Uncategorized → Forms |
comment:4 by , 18 months ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 6 comment:5 by , 18 months ago
It seems if we don't want to add some codes like the above, we have to change the flow of creation of the default renderer's engine. The default engine is not included in the
engines.all()
@MariuszFelisiak
Do you think it is worth changing the flow? I suggest letting it be solved in the simplest way.
follow-up: 8 comment:6 by , 18 months ago
Replying to Amir Karimi:
It seems if we don't want to add some codes like the above, we have to change the flow of creation of the default renderer's engine. The default engine is not included in the
engines.all()@MariuszFelisiak
Do you think it is worth changing the flow? I suggest letting it be solved in the simplest way.
I'm not sure what are you proposing. We didn't reject any proposition.
comment:7 by , 18 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 18 months ago
Replying to Mariusz Felisiak:
Replying to Amir Karimi:
It seems if we don't want to add some codes like the above, we have to change the flow of creation of the default renderer's engine. The default engine is not included in the
engines.all()@MariuszFelisiak
Do you think it is worth changing the flow? I suggest letting it be solved in the simplest way.
I'm not sure what are you proposing. We didn't reject any proposition.
Yeah. I just wanted to think about its optimal solution. I concluded that maybe there is no cleaner way to solve this issue than simply coding as suggested.
comment:10 by , 17 months ago
Has patch: | set |
---|
comment:11 by , 17 months ago
Needs tests: | unset |
---|---|
Patch needs improvement: | set |
comment:12 by , 17 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Thanks for the report. I was able to reproduce this issue.