#22557: staticfiles.json keeps deleted entries when collectstatic is run
-------------------------+-------------------------------------------------
Reporter: | Owner: nobody
tedtieken | Status: new
Type: Bug | Version: 1.7-beta-2
Component: | Keywords: hashed_files, collectstatic,
contrib.staticfiles | manifest
Severity: Normal | Has patch: 0
Triage Stage: | UI/UX: 0
Unreviewed |
Easy pickings: 0 |
-------------------------+-------------------------------------------------
There is some surprising behavior when the new HashedFilesMixin is used
with the new ManifestFilesMixin.
When you run {{{manage.py collectstatic --clear}}}:
1) a copy of the old manifest.json is loaded from disk into memory at
{{{ManifestFilesMixin.hashed_files}}}. This happens at the very beginning
of collectstatic (when {{{ManifestFilesMixin.__init__}}} is called, which
happens when the storage is initialized, which happens during
{{{Command.__init__}}})
2) the old manifest.json is deleted (which would lead most people to
believe the old manifest information is deleted as well)
3) new files are added to the {{{ManifestFilesMixin.hashed_files}}} dict,
and updated files get their records updated. But, this is building on top
of the last version of manifest.json. Keys for deleted files are never
removed and the deleted file mappings persist in the new manifest.json
which gets written back to disk at the end of collectstatic's post_process
phase.
There are at least a few problems caused by the current setup:
1) It can lead to hard-to-find asset bugs. If you rename the mypage.css
file to mynewpage.css, but forget to update all of the templates, the {{{
{% static %} }}} template tag will happily serve you the stale cache
record. This could lead to submarine bugs that aren't caught until
production as few sites have full enough test suites to catch missing css.
And the issue could get buried even deeper if you concatenate your CSS.
2) staticfiles.json has a memory leak. New files get added, but deleted
files never get deleted unless you physically delete the staticfiles.json
record.
3) If you write code that does anything with
{{{ManifestFilesMixin.hashed_files}}}, you can't trust the cache and you
have no way of cleaning it up when you find cache misses. (how I found
the bug) I'm writing a css concatenator and because staticfiles.json never
gets cleared, I have to keep a separate manifest for my mappings.
This problem may also exist when you use the CachedFilesMixin. I haven't
tested that, but a quick read over the code suggests similar behavior
exists there. 2 is less of an issue because caches are built to drop data
over time. 3 is less of an issue because you can easily delete stale cache
keys from anywhere in your application.
I see a few ways to fix the problem, and I'm sure there are others:
Option 1: clear the hashed files cache when collectstatic is run with
--clear.
If the code in
{{{django.contrib.staticfiles.management.commands.collectstatic}}} on
lines 88-89 is changed from:
{{{
if self.clear:
self.clear_dir('')
}}}
to
{{{
if self.clear:
self.clear_dir('')
if hasattr(self.storage, "hashed_files"):
self.storage.hashed_files.clear()
}}}
This is pretty simple when you're using a manifest but, when a cache
backend is used without a separate cache defined for staticfiles, this
looks like it will clear _THE ENTIRE_ default cache.
Option 2: Move the cleanup work into the StaticFilesStorage, add a call
from collectstatic.
If we added something like an {{{on_collectstatic}}} method to staticfiles
storages and call it during collectstatic, we could have better
encapsulation of this kind of cleanup that is also more futureproof.
in {{{django.contrib.staticfiles.management.commands.collectstatic}}} add
the following somewhere before post_process (~ line 107):
{{{
if hasattr(self.storage, "on_collectstatic"):
self.storage.on_collectstatic(self)
}}}
Which would allow us to add to the ManifestFilesMixin:
{{{
def on_collectstatic(command_object, *args, **kwargs):
if command_object.clear:
self.hashed_files = OrderedDict()
}}}
This lets us to do manifest files related cleanup work in the
ManifestFilesMixin, and anyone who needs to can do custom cleanup of their
own storage.
Option 3: add a warning to the documentation,
Of the options, I like some version of 2 the best.
I don't think this is necessarily a release blocker, but as soon as people
start using the ManifestFilesMixin they're going to start bumping into
this whether they realize it or not.
--
Ticket URL: <https://code.djangoproject.com/ticket/22557>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/052.4b6ed73a116f0237fabe3de92965e133%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
