#34773 closed Bug (fixed)
settings.STATICFILES_STORAGE does not return correct value when STORAGES are defined
Reported by: | Petr Dlouhý | Owned by: | Mariusz Felisiak |
---|---|---|---|
Component: | contrib.staticfiles | Version: | 4.2 |
Severity: | Release blocker | Keywords: | storages staticfiles |
Cc: | Jarosław Wygoda, Carlton Gibson | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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.
Change History (10)
comment:1 by , 16 months ago
Cc: | added |
---|---|
Severity: | Normal → Release blocker |
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 16 months ago
Petr, Does the following patch work for you?
-
django/conf/__init__.py
diff --git a/django/conf/__init__.py b/django/conf/__init__.py index da461ecc02..09ba791f6b 100644
a b class Settings: 217 217 os.environ["TZ"] = self.TIME_ZONE 218 218 time.tzset() 219 219 220 if self.is_overridden(" DEFAULT_FILE_STORAGE"):221 if self.is_overridden(" STORAGES"):220 if self.is_overridden("STORAGES"): 221 if self.is_overridden("DEFAULT_FILE_STORAGE"): 222 222 raise ImproperlyConfigured( 223 223 "DEFAULT_FILE_STORAGE/STORAGES are mutually exclusive." 224 224 ) 225 warnings.warn(DEFAULT_FILE_STORAGE_DEPRECATED_MSG, RemovedInDjango51Warning) 226 227 if self.is_overridden("STATICFILES_STORAGE"): 228 if self.is_overridden("STORAGES"): 225 if self.is_overridden("STATICFILES_STORAGE"): 229 226 raise ImproperlyConfigured( 230 227 "STATICFILES_STORAGE/STORAGES are mutually exclusive." 231 228 ) 229 setattr(self, 'DEFAULT_FILE_STORAGE', self.STORAGES.get(DEFAULT_STORAGE_ALIAS, {}).get("BACKEND")) 230 setattr(self, 'STATICFILES_STORAGE', self.STORAGES.get(STATICFILES_STORAGE_ALIAS, {}).get("BACKEND")) 231 if self.is_overridden("DEFAULT_FILE_STORAGE"): 232 warnings.warn(DEFAULT_FILE_STORAGE_DEPRECATED_MSG, RemovedInDjango51Warning) 233 if self.is_overridden("STATICFILES_STORAGE"): 232 234 warnings.warn(STATICFILES_STORAGE_DEPRECATED_MSG, RemovedInDjango51Warning) 233 235 234 236 def is_overridden(self, setting):
comment:3 by , 16 months ago
@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.
follow-up: 5 comment:4 by , 16 months ago
@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)
comment:5 by , 16 months ago
Cc: | added |
---|
Replying to Petr Dlouhý:
@Mariusz Felisiak
Now I realized, that there is another problem with this approach.
If I setSTORAGES["staticfiles"]["BACKEND"]="storages.backends.s3boto3.S3Boto3Storage"
with some OPTIONS and I set theAWS_*
variables at the same time those variables will be taken preferentially.
That means, that I can't setAWS_*
variables if I use the newSTORAGES
, 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 theget_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.
comment:6 by , 16 months ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 16 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
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 whenSTORAGES
is defined.Bug in 32940d390a00a30a6409282d314d617667892841.