Implement autoreload behaviour for cached template loader

[Jaap Roes]

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
    }
}]

[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!

[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.

[Tim Graham]

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

[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.

[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.

[Tim Graham]

​django-developers discussion

[Tim Graham]

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.

[Tom Forbes]

With my work on #27685 this should be easy to do. I've implemented a generic autoreload_started and file_changed signal, and the i18n package uses this to reset it's translation cache when a .mo file changes. We could implement similar logic here to call reset() on the cached loader when a template changes?

[Tom Forbes]

Patch: ​https://github.com/django/django/pull/10870

I implemented an autoreloader hook for this, but I felt that simply not caching templates when DEBUG is True was far simpler and hits the majority use case.

[Tim Graham]

Can you explain what you mean by the "majority use case"? The benefit seems to be that you can use the same TEMPLATES settings in development and production (although #25788 already helps with that). The downside is that you can no longer get the speed benefit of the cached template loader in development.

[Tom Forbes]

By the majority use case I meant people that simply want to have the same template settings across production and development settings, rather than duplicating pretty much the same configuration. We do loose the speed benefit but we never really had it in the first place as (I believe) very few people used the cached loader in development due to this very issue. That being said, having re-read the comments here I'm not sure if that really is the majority use case or just me (in my experience development is fast enough, but the two slightly different settings is annoying).

The autoreloader is flexible enough to subscribe to template changes, but the issue is that it's really not simple to do at the cached template loader level as everything is pretty abstract (and quite rightly so). Also we run into an issue where the loaders are not actually imported until the first template is rendered which means that the initial autoreloader started signal is not correctly registered.

I will see if I can find a better implementation, but the import issue is especially annoying.

[Tim Graham]

I'm not convinced that disabling the cached template loader when debug=True is much improvement and really solves this ticket.

[Jaap Roes]

I want to add that not having some form template caching during development may cause performance issues that only happen during development. This might cause a developer to try to mitigate the perceived slowness by rolling their own form of template caching. A real world example of this happening is this wagtail issue: ​https://github.com/wagtail/wagtail/pull/3077

[Tom Forbes]

Second attempt at a patch: ​https://github.com/django/django/pull/12928

[Carlton Gibson]

The basics of the patch seems small and simple here so +1. I'm just going to mark it PNI whilst we discuss bootstrapping the registration with the autoloader on the PR.

[Tom Forbes]

I'm not sure what to do about the registrations. I don't hate the way I've currently done it, and I don't really see it as a big problem to be honest. We just have an autoreload module imported in the django.template namespace 🤷‍♀️.

I'm happy to do it another way, but I can't think of one and nothing has been suggested so far. So I'm going to unmark this as PNI for now.

[Carlton Gibson <carlton@…>]

In 29845ecf:

Refs #25791 -- Added get_dirs() method to cached template loader.

[Carlton Gibson <carlton@…>]

In 658bcc1:

Fixed #25791 -- Implement autoreload behaviour for cached template loader.