[Django] #35868: 'collectstatic' management command inappropriately eating AttributeError exceptions

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+-------------------------------------
Reporter: codexterous | Type: Bug
Status: new | Component:
| contrib.staticfiles
Version: 5.0 | Severity: Normal
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Recently, Django 5.0 [https://docs.djangoproject.com/en/5.1/releases/5.0
/#features-removed-in-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.

The code in
'django/contrib/staticfiles/management/commands/collectstatic.py' reads as
follows:

{{{#!python
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 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35868>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+-------------------------------------

Reporter: Brett G | Owner: (none)

Type: Bug | Status: new
Component: contrib.staticfiles | Version: 5.0

Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Description changed by Brett G:

Old description:

New description:

Recently, Django 5.0 [https://docs.djangoproject.com/en/5.1/releases/5.0
/#features-removed-in-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:

{{{#!python
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.

--

--
Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:1>

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+-------------------------------------
Reporter: Brett G | Owner: (none)
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Description changed by Brett G:

Old description:

> Recently, Django 5.0 [https://docs.djangoproject.com/en/5.1/releases/5.0
> /#features-removed-in-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

Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:2>

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+------------------------------------

Reporter: Brett G | Owner: (none)
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 5.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

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 have
`get_modified_time` if folks are following the docs.
I believe it is safe to remove this
--
Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:3>

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+------------------------------------

Reporter: Brett G | Owner: pruszel
Type: Bug | Status: assigned

Component: contrib.staticfiles | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+------------------------------------

Changes (by pruszel):

* owner: (none) => pruszel
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:4>

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+------------------------------------
Reporter: Brett G | Owner: pruszel
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+------------------------------------
Changes (by pruszel):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:5>

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions

-------------------------------------+-------------------------------------
Reporter: Brett G | Owner: Peter
| Ruszel

Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 5.0
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:6>

[Django]

#35868: 'collectstatic' management command inappropriately eating AttributeError
exceptions
-------------------------------------+-------------------------------------
Reporter: Brett G | Owner: Peter
| Ruszel

Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 5.0
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce <42296566+sarahboyce@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"cf9da6fadd44cc7654681026d202387022b30d8d" cf9da6f]:
{{{#!CommitTicketReference repository=""
revision="cf9da6fadd44cc7654681026d202387022b30d8d"
Fixed #35868 -- Removed unneeded AttributeError catching in
collectstatic's delete_file().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35868#comment:7>