Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#25553 closed New feature (wontfix)

Add lazy version of admin_static's static

Reported by: David Sanders Owned by: nobody
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

To be CDN-friendly (with hashed static file names) it's important to run form assets through the static templatetag to get the correct filename. This can be done via django.contrib.admin.templatetag.admin_static.static, but if any chain of imports leads to static being invoked during a management command when the staticfiles are inaccessible (e.g. before collectstatic is performed) then it will fail to find the file and the command will error.

Lazy version can be used in these situations to lazily get the correct filename for form assets so that it is not resolved during management commands.

Attachments (1)

lazy_admin_static_static.patch (905 bytes) - added by David Sanders 7 years ago.

Download all attachments as: .zip

Change History (6)

Changed 7 years ago by David Sanders

comment:1 Changed 7 years ago by Tim Graham

I don't see why you would execute a management command (presumably in production) before finishing your deployment which should involve running collectstatic? Could you explain a bit more and expand on the rationale for this living in Django core and not in your app that requires the seemingly unusual use case?

comment:2 Changed 7 years ago by David Sanders

I could see the need to execute a management command before collectstatic, such as manage.py migrate --dry-run to see which migrations will be applied.

However, the bigger concern is that collectstatic is itself a management command so it can't run either.

If there's a situation where a models.py file imports a file which uses the non-lazy static, this causes the management command (including collectstatic) to fail since it is unable to find the path to the collected static since it hasn't been collected yet.

Currently the static templatetag in admin_static contains fallback logic if django.contrib.staticfiles isn't in INSTALLED_APPS. This is useful for thirdparty apps which can't rely on it being installed. While Django only uses it in a few admin files it is useful anywhere that uses form assets to get this same fallback logic. Using it for regular forms increases the odds this type of scenario will occur where it is evaluated during model collection in the app registry.

The docs don't really touch on using the static templatetag to ensure form assets have the correct filename, but it's important for having them work well with CachedStaticFilesStorage or ManifestStaticFilesStorage out of the box, otherwise they use unhashed filenames and may get cached (CDN or browser) without the developer realizing.

Long winded, but just trying to make the use case clear.

comment:3 Changed 7 years ago by Tim Graham

Resolution: wontfix
Status: newclosed

Problem acknowledged -- it looks like #21221 describes the same issue. As I don't think the admin static template tag is the right place to address the issue I'll close this ticket as "wontfix" for that approach, and we can continue the discussion about a possible solution on the other ticket. Maybe a solution could be for the BaseForm.media property to do the static transformation instead?

comment:4 in reply to:  3 Changed 7 years ago by David Sanders

Replying to timgraham:

Problem acknowledged -- it looks like #21221 describes the same issue. As I don't think the admin static template tag is the right place to address the issue I'll close this ticket as "wontfix" for that approach, and we can continue the discussion about a possible solution on the other ticket. Maybe a solution could be for the BaseForm.media property to do the static transformation instead?

Fair enough. I figured this was a known issue (since many of the admin forms use the static template tag to get around it), so this was more of a quick fix patch for a corner case. Ideally BaseForm.media should do it so that it just works out of the box, but since #21221 is over 2 years old it doesn't seem like there's much motivation to fix the underlying issue.

comment:5 Changed 6 years ago by Andromeda Yelton

FYI, I ran across this ticket when I found myself also needing a lazy version of static() thusly:

  • A model property used static() to grab the URLs of images used to illustrate model instances (they're icons representing model field choices);
  • Then I pushed to Heroku, where I'm using Whitenoise (per copy/paste from Heroku docs) to serve static images;
  • The app_config.import_models(all_models) call failed because the static() call in the model did not yet resolve to anything because collectstatic hadn't yet been run, so the push was rejected;
  • However, I can't run collectstatic until after the push makes it onto Heroku!

I've now hardcoded my URLs (so it works, but I feel like a bad person, having added a smidgen more technical debt to the world).

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