Opened 6 days ago
Closed 4 days ago
#35868 closed Bug (fixed)
'collectstatic' management command inappropriately eating AttributeError exceptions
Reported by: | Brett G | Owned by: | Peter Ruszel |
---|---|---|---|
Component: | contrib.staticfiles | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
Recently, Django 5.0 removed the django.utils.timezone.utc
alias to datetime.timezone.utc
.
I've written a custom file storage class that implements a get_modified_time
method which, predictably, used the django.utils.timezone.utc
alias. Use of this alias raises an AttributeError
in Django 5.0.
The code in 'django/contrib/staticfiles/management/commands/collectstatic.py' reads as follows:
try: # When was the target file modified last time? target_last_modified = self.storage.get_modified_time(prefixed_path) except (OSError, NotImplementedError, AttributeError): # The storage doesn't support get_modified_time() or failed pass else: try: # When was the source file modified last time? source_last_modified = source_storage.get_modified_time(path) except (OSError, NotImplementedError, AttributeError): pass
I assume the exception catch there is meant to ignore AttributeError
exceptions from custom file storage classes that do not implement get_modified_time
. This is inappropriate - custom file storage classes can be written by users, and the get_modified_time
method may raise AttributeError
for a completely unrelated reason.
The net effect of this is that due to the AttributeError
raised by my own get_modified_time
method, when collectstatic
was run, all static files were copied, regardless of modification time, as collectstatic
silently ignored this error. This was difficult to pin down.
Checking for the presence of a get_modified_time
attribute using duck typing (eg. hasattr(self.storage, 'get_modified_time')
) may be more appropriate than eating such a generic exception.
Change History (7)
comment:1 by , 6 days ago
Description: | modified (diff) |
---|
comment:2 by , 6 days ago
Description: | modified (diff) |
---|
comment:3 by , 5 days ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 4 days ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:5 by , 4 days ago
Has patch: | set |
---|
comment:6 by , 4 days ago
Triage Stage: | Accepted → Ready for checkin |
---|
Looks like the catching of the
AttributeError
was added back in 4f1ac8f5f15f08f34dc06c7442cec50a7af31c45 (which was to align to a change in django-storages https://github.com/jezdez/django-staticfiles/commit/b87a3531fbf4f18fea0633310410f8710cd0de39)As a custom storage backend should inherit from
django.core.files.storage.Storage
(and this is documented https://docs.djangoproject.com/en/5.1/howto/custom-file-storage/#how-to-write-a-custom-storage-class), I feel like it will always haveget_modified_time
if folks are following the docs.I believe it is safe to remove this