Admin: install debug_toolbar

[humitos]

This application needs to be installed if we want to enable it later in the web-extra instance. Otherwise, it fails due the static files are not in the manifest.

By installing it by default, collectstatic will find this application and copy these static files and add them in the manifest.

Note this application will only be used if SHOW_DEBUG_TOOLBAR=True, which is False by default.

[agjohnson]

I'm not familiar with exactly what debug-toolbar adds, but is there any concerns of parts of debug-toolbar leaking out to the prod application? I'll defer to you all here, but I'm guessing the added views/URLs are the largest concern, which we'd avoid in prod already.

It does seems a little odd to need this on prod, but the only other solution would be doing a collectstatic for prod and a separate collectstatic just for web-extra?

[humitos]

I'm not familiar with exactly what debug-toolbar adds, but is there any concerns of parts of debug-toolbar leaking out to the prod application? I'll defer to you all here, but I'm guessing the added views/URLs are the largest concern, which we'd avoid in prod already.

I think installing an app without adding middlewares or URLs is safe. Isn't it?

It does seems a little odd to need this on prod, but the only other solution would be doing a collectstatic for prod and a separate collectstatic just for web-extra?

I thought about this, but... I suppose this could break since web-extra may not have the same applications installed than regular webs and then those static files won't be available for web.

[agjohnson]

I think installing an app without adding middlewares or URLs is safe. Isn't it?

This is a good question. Looking at the application initialization, it does have side effects:

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

This seems to start initialization of the "panels". I'm not familiar enough with debug-toolbar internals to answer if this has zero side-effects without the middleware.

[agjohnson]

I'm guessing there won't be anything we can do that makes me feel 100% okay with installing debug applications in prod. If anyone else wants to overrule me here, I'll defer to anyone with more background on debug-toolbar.

[humitos]

Another option could be to do something like:

# settings.py

if "collectstatic" in cmd or RTD_SHOW_DEBUG_TOOLBAR :
  app.append("debug_toolbar")

That way we will install it only when running collectstatic or when we force it from the admin instance. Would this be an acceptable option? 🤔

[agjohnson]

Hah nice! If it's possible to do that, that does seem fine. I think as long as the prod web process doesn't evaluate debug toolbar, we're safe 👍

[benjaoming]

That way we will install it only when running collectstatic or when we force it from the admin instance. Would this be an acceptable option? thinking

In which cases are we running collectstatic + want DjDT assets + don't have DjDT enabled? Is it possible to do this only when RTD_SHOW_DEBUG_TOOLBAR is configured?

What I mean:

if RTD_SHOW_DEBUG_TOOLBAR :
    app.append("debug_toolbar")

[humitos]

In which cases are we running collectstatic + want DjDT assets + don't have DjDT enabled? Is it possible to do this only when RTD_SHOW_DEBUG_TOOLBAR is configured?

We run collectstatic from the web application which does not have debug_toolbar installed. That means that assets are not copied. Then we run an instance from web-extra that has the debug_toolbar installed. As the assets are shared between all the instances, the assets for debug_toolbar are not in S3 at this point.

So, we need to have debug_toolbar installed at the moment we are running collectstatic.

[humitos]

I changed the pattern here for a safer one.

I created a "static files finder" to always copy the static files from debug_toolbar, even if it's not on INSTALLED_APPS. This way, no matter from where we execute the collectstatic command, it will always copy these files --which is exactly what we want and it's safe to do 👍🏼

I tested it locally, and I ended up with the debug_toolbar/{css,js} folders in the MinIO, 💯

[benjaoming]

This sounds like a great solution 👍

[agjohnson]

Oooo, this is a really nice solution 💯

This seems much safer 👍

[humitos]

The docs-dev is failing, but I haven't change anything related to that, so 🤷🏼 . I'm merging this PR because we need it for today's deploy. We can come back later and fix the docs issue.

[benjaoming]

As noted in #9791:

I was going through our PRs since #9734 and this is one of the few PRs that doesn't change our docs.

It didn't, however, skip the documentation build:

https://readthedocs.org/projects/dev/builds/18765036/

But it did contain the feature:

readthedocs.org/.readthedocs.yml

Line 26 in 8ca4257

if [ "$READTHEDOCS_VERSION_TYPE" = "external" ] && git diff --quiet origin/main -- docs/;