Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False postive for debug_toolbar.W006 when manually declaring cached template loader. #1780

Closed
carltongibson opened this issue May 16, 2023 · 4 comments · Fixed by #1783
Closed

Comments

@carltongibson
Copy link
Contributor

carltongibson commented May 16, 2023

Hi.

Manually declare template loaders, using the cached loader:

TEMPLATES = [
    {
        "BACKEND": "django.template.backends.django.DjangoTemplates",
        "DIRS": [],
        # "APP_DIRS": True,
        "OPTIONS": {
            "context_processors": [
                "django.template.context_processors.debug",
                "django.template.context_processors.request",
                "django.contrib.auth.context_processors.auth",
                "django.contrib.messages.context_processors.messages",
            ],
            "loaders": [
                (
                    "django.template.loaders.cached.Loader",
                    [
                        "django.template.loaders.filesystem.Loader",
                        "django.template.loaders.app_directories.Loader",
                    ]
                ),
            ],
        },
    },
]

The DDT APP_DIRS check then gives a false positive:

?: (debug_toolbar.W006) At least one DjangoTemplates TEMPLATES configuration needs to use django.template.loaders.app_directories.Loader or have APP_DIRS set to True.
        HINT: Include django.template.loaders.app_directories.Loader in ["OPTIONS"]["loaders"]. Alternatively use APP_DIRS=True for at least one django.template.backends.django.DjangoTemplates backend configuration.

In:

https://github.com/jazzband/django-debug-toolbar/blob/de7e19c5ef08445ca592c75dfa3dad22c04992a1/debug_toolbar/apps.py#L26

It looks like checking for and walking loader.loaders would resolve this. 🤔

@matthiask
Copy link
Member

matthiask commented May 16, 2023

Hey

Thanks for the reoprt! This came up already; if OPTIONS["debug"] defaults to DEBUG; and if debug is False the cached loader is used automagically.

See https://docs.djangoproject.com/en/4.2/releases/1.11/#miscellaneous

The cached template loader is now enabled if OPTIONS['loaders'] isn’t specified and OPTIONS['debug'] is False (the latter option defaults to the value of DEBUG). This could be backwards-incompatible if you have some template tags that aren’t thread safe.

See #1550 and #1704 too.

We probably should still fix this but it has been declared as low priority in the past.

@carltongibson
Copy link
Contributor Author

carltongibson commented May 16, 2023

Yep, agreed. I don't think it's a biggie. (It's pretty niche.)

Current work around is:

SILENCED_SYSTEM_CHECKS = [
    "debug_toolbar.W006",  # See jazzband/django-debug-toolbar#1780
]

(I may be able to swing to it at some point, soonish)

@matthiask
Copy link
Member

matthiask commented May 16, 2023

Yes, or you (or any readers of this issue, not writing this for you specifically but for those who are drive-by-copy-pasting) could remove the loaders entry in the TEMPLATES configuration if you're fine with not using the cached loader when debugging.

This would be sufficient:

TEMPLATES = [
    {
        "BACKEND": "django.template.backends.django.DjangoTemplates",
        "DIRS": [],
        "APP_DIRS": True,
        "OPTIONS": {
            "context_processors": [
                "django.template.context_processors.debug",
                "django.template.context_processors.request",
                "django.contrib.auth.context_processors.auth",
                "django.contrib.messages.context_processors.messages",
            ],
        },
    },
]

(It's what I use these days.)

@carltongibson
Copy link
Contributor Author

Yep 👍.

Since the autoreloader support for templates was added in Django 4.1, the cached loader is used in development too. As such, there's almost no need to declare loaders for the majority case.

(I'm only doing it myself because I'm tweaking...)

carltongibson added a commit to carltongibson/django-debug-toolbar that referenced this issue May 16, 2023
…d template loaders.

Django's cached loader wraps a list of child loaders, which may
correctly contain the required app directories loader.
carltongibson added a commit to carltongibson/django-debug-toolbar that referenced this issue May 16, 2023
…d template loaders.

Django's cached loader wraps a list of child loaders, which may
correctly contain the required app directories loader.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants