Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32744 closed Bug (fixed)

Template changes cause dev server to reload

Reported by: Ryan P Kilby Owned by: Hasan Ramezani
Component: Template system Version: 3.2
Severity: Release blocker Keywords: autoreload
Cc: Ryan P Kilby, Tom Forbes Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Django 3.2 has changed the autoreload behavior of the dev server, and it now reloads on template file changes. Reverting to 3.1 fixes the issue. I believe this is related to #25791 and ​https://github.com/django/django/pull/12928

Template settings:

DEBUG = True

TEMPLATES = [
    {
        "BACKEND": "django.template.backends.django.DjangoTemplates",
        "DIRS": [os.path.join(BASE_DIR, "templates")],
        "APP_DIRS": True,
        "OPTIONS": {
            "debug": DEBUG,
            "context_processors": [
                "django.template.context_processors.debug",
                "django.template.context_processors.request",
                "django.contrib.auth.context_processors.auth",
                "django.contrib.messages.context_processors.messages",
            ],
        },
    },
]

Given that it can take several seconds for the dev server to reload, this change can be disruptive to template authoring.

Attachments (1)

ticket32744.zip (15.2 KB ) - added by Carlton Gibson 3 years ago.
Barest project NOT reproducing editing the hello.html file whilst running the dev server.

Download all attachments as: .zip

Change History (13)

comment:1 by Ryan P Kilby, 3 years ago

Cc: Ryan P Kilby added

comment:2 by Carlton Gibson, 3 years ago

Cc: Tom Forbes added

comment:3 by Carlton Gibson, 3 years ago

Resolution: β†’ worksforme
Status: new β†’ closed

Hey Ryan πŸ‘‹

Thanks for the report. I'm need more detail, as I'm struggling to reproduce.

Testing running with Django 3.2.3, both with Django's built-in runserver and Channels' ASGI based one too. I'm not seeing the auto-reload behaviour on template saves.

My TEMPLATES looks almost identical to yours.

Obviously something is going on, but there must be more to it…

Watching for file changes with StatReloader

What reloader are you using? πŸ€”

comment:4 by Carlton Gibson, 3 years ago

Resolution: worksforme β†’ needsinfo

Having seen the duplicate #32745 in the timeline, I've tried again here (double checking). It's still not reproducing with a fresh project so, still more info... πŸ€”

I'll upload a sample project with just a single view and template.

Observed: startproject, startapp, then app template and view. runserver, edit the template. Not reload.

Clearly you're seeing something, but what?
Thanks.

by Carlton Gibson, 3 years ago

Attachment: ticket32744.zip added

Barest project NOT reproducing editing the hello.html file whilst running the dev server.

comment:5 by Ryan P Kilby, 3 years ago

Resolution: needsinfo
Status: closed β†’ new

Hi Carlton,
long time :)

Figured out how to reproduce the issue:

  • Ensure TEMPLATES["DIRS"] contains a directory path that's a string instead of a Path object.
  • Add a template to that directory, point a view to that template.
  • Modify/save the template, observe reload

Specifically, the failure is occurring in the ​template_changed handler's if template_dir in file_path.parents check. As template_dir is a string, it fails the comparison to the parent Paths. While the failure occurs here, I'm guessing that loader.get_dirs should normalize its directories to Paths.

Also, you'd only hit this issue when upgrading an older settings module. New projects are setup to use Paths, while old projects would be building paths with os.path.join().

comment:6 by Mariusz Felisiak, 3 years ago

Keywords: autoreload added
Severity: Normal β†’ Release blocker
Triage Stage: Unreviewed β†’ Accepted

Thanks for extra details, I'm was able to reproduce this issue with

  • 'DIRS': ['template_dir'],
  • 'DIRS': [Path('template_dir')], and
  • 'DIRS': ['/full/path/to/template_dir'].

I think we should normalize directories to resolved Paths, e.g.

diff --git a/django/template/autoreload.py b/django/template/autoreload.py
index 36952ef9aa..6a648ce0c3 100644
--- a/django/template/autoreload.py
+++ b/django/template/autoreload.py
@@ -4,6 +4,7 @@ from django.template.backends.django import DjangoTemplates
 from django.utils.autoreload import (
     autoreload_started, file_changed, is_django_path,
 )
+from django.utils._os import to_path
 
 
 def get_template_directories():
@@ -15,13 +16,13 @@ def get_template_directories():
         if not isinstance(backend, DjangoTemplates):
             continue
 
-        items.update(backend.engine.dirs)
+        items.update(to_path(dir).resolve() for dir in backend.engine.dirs)
 
         for loader in backend.engine.template_loaders:
             if not hasattr(loader, 'get_dirs'):
                 continue
             items.update(
-                directory
+                to_path(directory).resolve()
                 for directory in loader.get_dirs()
                 if not is_django_path(directory)
             )

Regression in 658bcc16f1b814b3a063d3fa16fabaea8b471863.

comment:7 by Hasan Ramezani, 3 years ago

Owner: changed from nobody to Hasan Ramezani
Status: new β†’ assigned

comment:8 by Hasan Ramezani, 3 years ago

Has patch: set

comment:9 by Carlton Gibson, 3 years ago

Triage Stage: Accepted β†’ Ready for checkin

comment:10 by Carlton Gibson, 3 years ago

Patch needs improvement: set
Triage Stage: Ready for checkin β†’ Accepted

comment:11 by Carlton Gibson <carlton.gibson@…>, 3 years ago

Resolution: β†’ fixed
Status: assigned β†’ closed

In 68357b2:

Fixed #32744 -- Normalized to pathlib.Path in autoreloader check for template changes.

comment:12 by Carlton Gibson <carlton.gibson@…>, 3 years ago

In c0d506f5:

[3.2.x] Fixed #32744 -- Normalized to pathlib.Path in autoreloader check for template changes.

Backport of 68357b2ca9e88c40fc00d848799813241be39129 from main

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