-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 positive for debug_toolbar.W006? #1550
Comments
I was able to confirm that the example app can run with You can silence this check by making use of the SILENCED_SYSTEM_CHECKS setting. |
We could make the check a bit smarter by inspecting |
I attempted to do this and discovered, that Django automatically adds the cached loader since 1.11 when So, there's no good reason to specify This only affected us since we have been cargo culting our settings for years and years now. I think it's fine to require an addition to I have a new suggestion to make: We could add the hint that people may just want to drop |
Thanks for doing the research on that!
It makes sense to include the information for devs to make a choice from. It's only a warning check and doesn't break anything so I'm in agreement that the requirement of needing to use |
You may be right about this, it looks like it was the cause of my problem too. I found the time to look into the error I was getting at deployment time and it was related to us manually wrapping the template loaders in the cached loader for production (obviously no longer needed). Deleting all template loader settings resolved both the debug_toolbar warning and the error. I apologise if my issue was in fact the red herring! Thank you, everybody. |
Don't worry, there definitely isn't a need to apologize. |
@matthiask it looks like that loader isn't included when debug = True, so it shouldn't matter for the toolbar. |
* Don't raise W006 warning when app loader is specified. Fixes #1550 * Update docs/changes.rst Co-authored-by: Matthias Kestenholz <mk@feinheit.ch>
Django now does caching by default with disabled debug, so there is no need ot do that as well. See django-commons/django-debug-toolbar#1550
https://django-debug-toolbar.readthedocs.io/en/latest/checks.html django-commons/django-debug-toolbar#1550 (comment) Based on the discussion in the linked thread, loader configuration was replaced with `"APP_DIRS": True`. Django adds required loaders by default, so there was no need to specify them directly.
After upgrading to v3.2.3 I get the system warning W006, and an advisement to set APP_DIRS=True for at least one of my template settings.
Shouldn't that check also be looking at the value of loaders in order to avoid false warnings?
For example, the project in question had the following template settings:
Django Debug Toolbar works wonderfully with these settings. My understanding is that the app_directories loader is able to find the templates that the toolbar needs. The W0006 warning seems to be a false postive here (but to be honest, it's been a couple of years since I created this config).
As a quick experiment I tried just deleting the loaders option and adding the suggested APP_DIRS setting. It resulted in a key error for 'django' during deployment while trying to run migrate - so I've reverted back to v3.2.2 for now.
The text was updated successfully, but these errors were encountered: