[Django] #34773: settings.STATICFILES_STORAGE does not return correct value when STORAGES are defined
Django
are defined
-------------------------------------+-------------------------------------
Reporter: Petr | Owner: nobody
Dlouhý |
Type: Bug | Status: new
Component: | Version: 4.2
contrib.staticfiles | Keywords: storages
Severity: Normal | staticfiles
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I transitioned to the `STORAGES` settings introduced in Django 4.2 and I
got `OfflineGenerationError` with `django-compressor`.
I tracked down the issue to be actually bug in `django-debug-toolbar`'s
`StaticFilesPanel` which patches the static files storage and doesn't
count with the new `STORAGES`. The whole issue is described here:
https://github.com/jazzband/django-debug-toolbar/issues/1823
The problem itself is, that `django-debug-toolbar` is using
`settings.STATICFILES_STORAGE` to get the storage class which returns
`'django.contrib.staticfiles.storage.StaticFilesStorage'` even if I have
`settings.STORAGES["staticfiles"]["BACKEND"]='whitenoise.storage.CompressedManifestStaticFilesStorage'`.
Since Django 4.2 requires `STATICFILES_STORAGE` to be deleted from
settings when `STORAGES` are present I don't have `STATICFILES_STORAGE`
set to any particular value.
Tracking down this whole issue took me whole week and was very difficult
to tackle, because it did appear in completely different place in the
system (missing records in the manifest files of `django-compressor`) than
the issue actually was (`StaticFilesPanel` in `django-debug-toolbar`).
I think this issue partially goes on behalf of Django itself. If Django
would return `None` value or throw an exception when accessing
`settings.STATICFILES_STORAGE` with `STORAGES` defined I would see the
issue immediately.
The `settings.STATICFILES_STORAGE` could also return the same value as
`settings.STORAGES["staticfiles"]["BACKEND"]`, but I am bigger fun of
throwing an exception, because this could lead to other kind of unexpected
behavior.
--
Ticket URL: <https://code.djangoproject.com/ticket/34773>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
are defined
Reporter: Petr Dlouhý | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
Keywords: storages staticfiles | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Jarosław Wygoda (added)
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report. IMO, we should make both settings exchangeable
during the deprecation period, so
`settings.STATICFILES_STORAGE`/`DEFAULT_FILE_STORAGE` should be updated
when `STORAGES` is defined.
Bug in 32940d390a00a30a6409282d314d617667892841.
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:1>
Django
are defined
--------------------------------------+------------------------------------
Reporter: Petr Dlouhý | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
Keywords: storages staticfiles | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Mariusz Felisiak):
Petr, Does the following patch work for you?
{{{#!diff
diff --git a/django/conf/__init__.py b/django/conf/__init__.py
index da461ecc02..09ba791f6b 100644
--- a/django/conf/__init__.py
+++ b/django/conf/__init__.py
@@ -217,18 +217,20 @@ class Settings:
os.environ["TZ"] = self.TIME_ZONE
time.tzset()
- if self.is_overridden("DEFAULT_FILE_STORAGE"):
- if self.is_overridden("STORAGES"):
+ if self.is_overridden("STORAGES"):
+ if self.is_overridden("DEFAULT_FILE_STORAGE"):
raise ImproperlyConfigured(
"DEFAULT_FILE_STORAGE/STORAGES are mutually
exclusive."
)
- warnings.warn(DEFAULT_FILE_STORAGE_DEPRECATED_MSG,
RemovedInDjango51Warning)
-
- if self.is_overridden("STATICFILES_STORAGE"):
- if self.is_overridden("STORAGES"):
+ if self.is_overridden("STATICFILES_STORAGE"):
raise ImproperlyConfigured(
"STATICFILES_STORAGE/STORAGES are mutually
exclusive."
)
+ setattr(self, 'DEFAULT_FILE_STORAGE',
self.STORAGES.get(DEFAULT_STORAGE_ALIAS, {}).get("BACKEND"))
+ setattr(self, 'STATICFILES_STORAGE',
self.STORAGES.get(STATICFILES_STORAGE_ALIAS, {}).get("BACKEND"))
+ if self.is_overridden("DEFAULT_FILE_STORAGE"):
+ warnings.warn(DEFAULT_FILE_STORAGE_DEPRECATED_MSG,
RemovedInDjango51Warning)
+ if self.is_overridden("STATICFILES_STORAGE"):
warnings.warn(STATICFILES_STORAGE_DEPRECATED_MSG,
RemovedInDjango51Warning)
def is_overridden(self, setting):
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:2>
Django
are defined
--------------------------------------+------------------------------------
Reporter: Petr Dlouhý | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
Keywords: storages staticfiles | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Petr Dlouhý):
@Mariusz Felisiak
This code started working only when I re-introduced additional settings
for the `DEFAULT_STORAGE` (which is
`"storages.backends.s3boto3.S3Boto3Storage"` in my case):
{{{
AWS_ACCESS_KEY_ID = AWS_DEFAULT.get("AWS_ACCESS_KEY_ID")
AWS_SECRET_ACCESS_KEY = AWS_DEFAULT.get("AWS_SECRET_ACCESS_KEY")
AWS_S3_ENDPOINT_URL = AWS_DEFAULT.get("AWS_S3_ENDPOINT_URL")
AWS_S3_HOST = AWS_DEFAULT.get("AWS_S3_HOST", "s3.amazonaws.com")
AWS_S3_CUSTOM_DOMAIN = AWS_DEFAULT.get("AWS_S3_CUSTOM_DOMAIN", None)
AWS_STORAGE_BUCKET_NAME = AWS_DEFAULT.get("AWS_STORAGE_BUCKET_NAME",
"bucket")
}}}
I already deleted those values from my settings (used them only for
`STORAGES`), but only now I realized, that I need them for applications
like `django-avatar` that didn't transition to the new `STORAGES` yet and
were appearing to work but in fact were working with different storage
than I intended.
I am not sure, if there are not any other consequences of this code, but
this seems to enable much smoother transition.
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:3>
Django
are defined
--------------------------------------+------------------------------------
Reporter: Petr Dlouhý | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
Keywords: storages staticfiles | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by Petr Dlouhý):
@Mariusz Felisiak
Now I realized, that there is another problem with this approach.
If I set
`STORAGES["staticfiles"]["BACKEND"]="storages.backends.s3boto3.S3Boto3Storage"`
with some OPTIONS and I set the `AWS_*` variables at the same time those
variables will be taken preferentially.
That means, that I can't set `AWS_*` variables if I use the new
`STORAGES`, so this approach will not help at all in my case.
I think mixing the old and new settings will lead to many unexpected
problems.
I would be OK if I would have to wait until the `get_storage_class` usage
is removed from all applications that I use.
So overall I still think the best approach to this issue would be to solve
this issue with:
{{{
setattr(self, 'DEFAULT_FILE_STORAGE', None)
setattr(self, 'STATICFILES_STORAGE', None)
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:4>
Django
are defined
--------------------------------------+------------------------------------
Reporter: Petr Dlouhý | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
Keywords: storages staticfiles | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* cc: Carlton Gibson (added)
Comment:
Replying to [comment:4 Petr Dlouhý]:
> @Mariusz Felisiak
> Now I realized, that there is another problem with this approach.
> If I set
`STORAGES["staticfiles"]["BACKEND"]="storages.backends.s3boto3.S3Boto3Storage"`
with some OPTIONS and I set the `AWS_*` variables at the same time those
variables will be taken preferentially.
> That means, that I can't set `AWS_*` variables if I use the new
`STORAGES`, so this approach will not help at all in my case.
Yes, but this is something that you should report to the 3rd-party
storage, it's not an issue in Django itself. I'd not expect that all 3rd-
party storage backends will support the new setting immediately.
Therefore, it's a pending deprecation, so there is plenty of time to
switch to the new `STORAGES`. I don't see anything unusual in using
`STORAGES` for defining the engine and project-settings for defining
options.
> I think mixing the old and new settings will lead to many unexpected
problems.
> I would be OK if I would have to wait until the `get_storage_class`
usage is removed from all applications that I use.
> So overall I still think the best approach to this issue would be to
solve this issue with:
> {{{
> setattr(self, 'DEFAULT_FILE_STORAGE', None)
> setattr(self, 'STATICFILES_STORAGE', None)
> }}}
I don't agree, this will cause extra work for all 3rd-party packages that
use the old settings, and may effectively force them to use `STORAGES`
immediately which is against our deprecation policy.
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:5>
Django
are defined
Reporter: Petr Dlouhý | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
staticfiles |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* owner: nobody => Mariusz Felisiak
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:6>
Django
are defined
-------------------------------------+-------------------------------------
Reporter: Petr Dlouhý | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
Keywords: storages | Triage Stage: Accepted
staticfiles |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/17170 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:7>
Django
are defined
-------------------------------------+-------------------------------------
Reporter: Petr Dlouhý | Owner: Mariusz
| Felisiak
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution:
staticfiles | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:8>
Django
are defined
-------------------------------------+-------------------------------------
Reporter: Petr Dlouhý | Owner: Mariusz
| Felisiak
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution: fixed
Keywords: storages | Triage Stage: Ready for
staticfiles | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"6b965c600054f970bdf94017ecf2e0e6e0a4326b" 6b965c6]:
{{{
#!CommitTicketReference repository=""
revision="6b965c600054f970bdf94017ecf2e0e6e0a4326b"
Fixed #34773 -- Fixed syncing DEFAULT_FILE_STORAGE/STATICFILES_STORAGE
settings with STORAGES.
Thanks Petr Dlouhý for the report.
Bug in 32940d390a00a30a6409282d314d617667892841.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:9>
Django
are defined
-------------------------------------+-------------------------------------
Reporter: Petr Dlouhý | Owner: Mariusz
| Felisiak
Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 4.2
Severity: Release blocker | Resolution: fixed
Keywords: storages | Triage Stage: Ready for
staticfiles | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d34db6602e89387ae9ddc9ae94defe5c838acb88" d34db660]:
{{{
#!CommitTicketReference repository=""
revision="d34db6602e89387ae9ddc9ae94defe5c838acb88"
[4.2.x] Fixed #34773 -- Fixed syncing
DEFAULT_FILE_STORAGE/STATICFILES_STORAGE settings with STORAGES.
Thanks Petr Dlouhý for the report.
Bug in 32940d390a00a30a6409282d314d617667892841.
Backport of 6b965c600054f970bdf94017ecf2e0e6e0a4326b from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34773#comment:10>