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

Use ManifestStaticFilesStorage for cache busting #648

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jarosenb
Copy link
Contributor

@jarosenb jarosenb commented May 25, 2023

Overview

By setting ManifestStaticFilesStorage as the backend, our static files will have hashed filenames. This will prevent stale content from being served to users when we update our styles. It will also prevent collectstatic from doing a no-op instead of overwriting files.

Related

@@ -289,6 +289,8 @@ def gettext(s): return s
('ui', os.path.join(BASE_DIR, 'taccsite_ui', 'dist')),
)

STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to become deprecated in Django 4.2, which we want to use soon. If so, would you recommend we note that somewhere (Jira) (code comment here)? The new way (not available until 4.2) would be via STORAGES setting.

@taoteg
Copy link
Collaborator

taoteg commented May 25, 2023

As long as we don't hit this bug: django-cms/djangocms-text-ckeditor#474

@wesleyboar
Copy link
Member

@taoteg, we ought not to. You brought us up to djangocms-text-ckeditor v3.10 and @jarosenb brought us up to v5. If that exists, that app needs help. I'll test.

@wesleyboar wesleyboar self-requested a review May 25, 2023 23:03
Copy link
Member

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, crap. It got through the CSS of ckeditor at least, but it died on one of Django's SVGs. I dunno what to do yet. This week has taken much of my brain already.

ValueError: The file 'cms/fonts/src/logo.svg' could not be found with <django.contrib.staticfiles.storage.ManifestStaticFilesStorage object at 0xffffb1ced6d0>.

root@core_cms:/code# python manage.py collectstatic
Found another file with the destination path 'admin/img/search.svg'. It will be ignored since only the first encountered file is collected. If this is not what you want, make sure every static file has a unique path.
Found another file with the destination path 'djangocms_text_ckeditor/ckeditor/contents.css'. It will be ignored since only the first encountered file is collected. If this is not what you want, make sure every static file has a unique path.
[…]
Found another file with the destination path 'utrc-cms/img/org_logos/utrc-horizontal-logo-white-simple.svg'. It will be ignored since only the first encountered file is collected. If this is not what you want, make sure every static file has a unique path.
Post-processing 'cms/css/3.7.4/cms.welcome.css' failed!

Traceback (most recent call last):
  File "manage.py", line 22, in <module>
    execute_from_command_line(sys.argv)
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/core/management/__init__.py", line 381, in execute_from_command_line
    utility.execute()
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/core/management/__init__.py", line 375, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/core/management/base.py", line 323, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/core/management/base.py", line 364, in execute
    output = self.handle(*args, **options)
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 188, in handle
    collected = self.collect()
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 134, in collect
    raise processed
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/storage.py", line 294, in _post_process
    content = pattern.sub(converter, content)
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/storage.py", line 193, in converter
    hashed_url = self._url(
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/storage.py", line 132, in _url
    hashed_name = hashed_name_func(*args)
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/storage.py", line 343, in _stored_name
    cache_name = self.clean_name(self.hashed_name(name))
  File "/opt/pysetup/.venv/lib/python3.8/site-packages/django/contrib/staticfiles/storage.py", line 93, in hashed_name
    raise ValueError("The file '%s' could not be found with %r." % (filename, self))
ValueError: The file 'cms/fonts/src/logo.svg' could not be found with <django.contrib.staticfiles.storage.ManifestStaticFilesStorage object at 0xffffb1ced6d0>.

@taoteg
Copy link
Collaborator

taoteg commented May 26, 2023

After making the setting changes in the host, did you rerun collect static to validate the manifest? python manage.py collectstatic --clear --noinput

@wesleyboar
Copy link
Member

wesleyboar commented May 26, 2023

@taoteg, Yes, I think: I used this branch, and confirmed setting on my host was synced to container. I ran that command with --clear --no-input and received the same output.

@wesleyboar wesleyboar added paused Started but not actively in progress priority ━ Medium priority labels Nov 13, 2023
@wesleyboar
Copy link
Member

I am still interested in this. I retain the branch and keep the PR open.

"By setting ManifestStaticFilesStorage as the backend [...] prevent stale content […]"
This is a familiar and good feature. I have used it in my pre-TACC CMS work.

"It will also prevent collectstatic from doing a no-op instead of overwriting files."
This would clean up log cruft too.

@daandeheij
Copy link

Are you guys still having issues with this? I am not sure if you are encountering the same issue as mentioned here. I just left a solution there, because somehow I couldn't find it anywhere online. Anyways, if you're having difficulties, I can probably help out if you want.

@wesleyboar
Copy link
Member

wesleyboar commented Mar 7, 2024

Hi, @daandeheij. Thank you for the reference!

I suspect our issue was not with CKEditor.

Because the log showed Post-processing 'cms/css/3.7.4/cms.welcome.css' failed! before the error — and that's a Django CMS v3.7.4 stylesheet.

I am still unsure of the reason, for failure of this initial attempt at ManifestStaticFilesStorage. When I have the bandwidth and requirement, I'll merge main and test again — maybe with newer syntax — because this repo has had major dependency upgrades and other relevant changes since.


P.R. Status Update

@jarosenb tried ManifestStaticFilesStorage this year also. It failed in Core-CMS, but worked in a client app. This project, an upstream CSS lib, and other clients, needed several changes to support that new solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
paused Started but not actively in progress priority ━ Medium priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants