Avoid top-level versioned_static calls #89
Conversation
makemigrations is the key check which may fail
Prevent versioned_static from being called too early
|
Thank you for this @tomkins |
| "staticfiles": { | ||
| "BACKEND": os.environ.get( | ||
| "STATICFILES_STORAGE", | ||
| "django.contrib.staticfiles.storage.StaticFilesStorage", |
There was a problem hiding this comment.
question: Is there a particular reason to make this an env var? Why not always use ManifestStaticFilesStorage?
There was a problem hiding this comment.
Testing with ManifestStaticFilesStorage requires that you've done collectstatic beforehand, for example:
STATICFILES_STORAGE=django.contrib.staticfiles.storage.ManifestStaticFilesStorage pytest
Fails with:
ValueError: Missing staticfiles manifest entry for 'wagtailadmin/css/core.css'
As the static files aren't in place. But if you do:
STATICFILES_STORAGE=django.contrib.staticfiles.storage.ManifestStaticFilesStorage ./testmanage.py collectstatic --no-input
STATICFILES_STORAGE=django.contrib.staticfiles.storage.ManifestStaticFilesStorage pytest
It works as expected - the static files are in place.
I've added the additional commands in tox.ini that will run collectstatic beforehand if you're running inside tox. If you're just doing quick development outside of tox - then it defaults to StaticFilesStorage - which is faster/easier if you need a quick feedback loop.
For most of our projects at DEV we try and have some form of CI that runs with ManifestStaticFilesStorage, as that's what we deploy with - so running CI with the same storage helps reduce any errors.
It's possibly a bit opinionated, but I've tried to follow something roughly along the lines of: https://github.com/wagtail/wagtail/blob/main/wagtail/test/settings.py#L68.
|
This all looks sensible to me, thanks @tomkins! One minor question for my own education but other than that, happy to get this in. |
Closes #88
To fully test - upload an image in the admin and check that the AI generation button still works.
I'm fairly confident in the media fix (it's similar to wagtail/wagtail#5694), however the CI testing might be slightly opinionated. My personal opinion - CI should run static file creation (webpack), manifest storage should be used to avoid missing files, and migrations should be tested in projects.
However - happy for feedback if this isn't what you want!