Opened 2 years ago

Last modified 17 months ago

#25791 new New feature

Implement autoreload behaviour for cached template loader

Reported by: Jaap Roes Owned by: nobody
Component: Template system Version: master
Severity: Normal Keywords: autoreload templates cached loader
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

It would be nice to be able get the speed benefit of the cached template loader during development, but without the downside of having to restart the server every time you change a template. It turns out it's possible with just a few changes.

Because it's not really possible to configure the cached template loader I did have to base this patch on the fix for #25788. Enabling autoreloading for the cached template loader would work like this:

TEMPLATES = [{
    'BACKEND': 'django.template.backends.django.DjangoTemplates',
    'DIRS': [os.path.join(BASE_DIR, 'templates')],
    'APP_DIRS': True
    'OPTIONS': {
        'cache_templates': True, 
        'autoreload': DEBUG
    }
}]

Change History (7)

comment:1 Changed 2 years ago by Tim Graham

The proposed API of passing template loader specific configuration in 'OPTIONS' doesn't seem like a clean separation of concerns. For example, any third-party template loaders desiring some configuration options aren't able to patch the DjangoTemplates backend similarly.

As for me, I don't see a big need to use the cached template loader during local development. How much does this actually improve performance? I'd like to see some confirmation on the DevelopersMailingList from other users that the additional complexity is worthwhile. Thanks!

comment:2 Changed 2 years ago by Jaap Roes

I'm proposing that optional template caching becomes first class behaviour of the Django template engine. Using the cached template loader is a good practice in general, so in my opinion it should be trivial to enable/disable it.

Having caching and auto reloading options on the Engine is actually on par with the jinja2 engine. Which for example has an auto_reload option (that defaults to settings.DEBUG) (https://github.com/django/django/blob/master/django/template/backends/jinja2.py#L31).

With auto reloading in place Django template caching could even be part of any fresh Django project created with startproject.

As it is now I've seen many cases where people either forget to turn on caching in production, or forget to turn off caching during development (which results in lots of head scratching and frustration).

Finally, using the Engine to configure the cached template loader is also a lot nice than the current way. The fact that you can configure a template loader by using a nesting of lists was introduced when the cached template loader landed:

https://github.com/django/django/commit/44b9076bbed3e629230d9b77a8765e4c906036d1#diff-eed563533d29c9e8e6f1273759b6314cR74

It doesn't seem to be documented, and it could probably be deprecated with this patch.

comment:3 Changed 2 years ago by Tim Graham

Triage Stage: UnreviewedSomeday/Maybe

Thanks for the additional explanation. Can you please run your proposal by the DevelopersMailingList so it reaches a wider audience?

comment:4 Changed 2 years ago by Preston Timmons

I think this is an interesting idea. I looked into something similar as part of previous template refactoring, although my plan was to have the engine handle caching and deprecate the cached loader. An initial patch is still up here:

https://github.com/prestontimmons/django/commit/4683300c60033ba0db472c81244f01ff932c6fb3

I found this opens the possibility to potentially confusing cases, though. For instance, if a template is cached for a path, but a newer template is placed on the filesystem in a directory that would otherwise have higher precedence, the cached template always win. This doesn't mean autoreload is a bad idea, but if implemented, it's not a good default for development.

I'll second Tim that OPTIONS is really meant for engine configuration, not loader configuration. I'm not sure we want to blur the lines this way.

comment:5 Changed 2 years ago by Jaap Roes

I think the issue you mention about template precedence is solvable, how does Jinja2 handle these cases? Jinja2's engine enables auto_reload if DEBUG is true (and a cache is set) so I'm assuming it will be turned on in development in many cases.

Since template (origin) invalidation is the responsibility of the loader (in both our implementations) it should (probably?) be possible to implement an is_stale/uptodate function that checks if the template is superseded.

Regarding loader configuration in the Engine options. The Jina2 engine passes all it's arguments into an Environment (http://jinja.pocoo.org/docs/dev/api/#jinja2.Environment) that is used by Jinja2's internals to configure all sorts of things. Django doesn't have the concept of an environment, but the engine is used in a similar way (in fact this commit https://github.com/django/django/commit/29a977ab143f549c8849e9f08ca5800830eaaabb#diff-2ec90d8b6d7f44124689729e42876209 specifically mentions that template loaders can look up configuration on the engine.

But, this isn't so much about configuring loaders as it is about making template caching a more prominent feature of the Django template engine. That template caching is currently possible by way of a quasi template loader is an implementation detail.

I'll try to write up something for the developers mailinglist today to get some more input.

comment:7 Changed 17 months ago by Tim Graham

Patch needs improvement: set
Triage Stage: Someday/MaybeAccepted

Aymeric suggests:

The proposed patch for auto-reloading doesn’t seem too bad in terms of complexity. Its main drawback is that it add a new auto-reload mechanism, distinct from django.utils.autoreload.

I would prefer to trigger the autoreloader on template file changes to make it easier to switch e.g. to watchman in the future.

Note: See TracTickets for help on using tickets.
Back to Top