settings.STATICFILES_STORAGE does not return correct value when STORAGES are defined

[Petr Dlouhý]

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.

[Mariusz Felisiak]

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.

[Mariusz Felisiak]

Petr, Does the following patch work for you?

django/conf/__init__.py

@@ -217 +217 @@
             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):

[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.

[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)

[Mariusz Felisiak]

Replying to 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.

[Mariusz Felisiak]

​PR

[GitHub <noreply@…>]

In 6b965c6:

Fixed #34773 -- Fixed syncing DEFAULT_FILE_STORAGE/STATICFILES_STORAGE settings with STORAGES.

Thanks Petr Dlouhý for the report.

Bug in 32940d390a00a30a6409282d314d617667892841.

[Mariusz Felisiak <felisiak.mariusz@…>]

In d34db660:

[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