fix: Django52 use STORAGES instead of DEFAULT_FILE_STORAGE and STATICFILES_STORAGE #2844

mubbsharanwar · 2025-08-25T07:30:20Z

Description

Depreciate STATICFILES_STORAGE and DEFAULT_FILE_STORAGE, and use STORAGE to upgrade edx-platform to Django 5.2

deborahgu · 2025-08-25T12:59:02Z

is this different from #2828 which has a pending question on it?

awais786 · 2025-08-26T15:17:10Z

is this different from #2828 which has a pending question on it?

Yes this is actual one

awais786 · 2025-08-27T13:08:57Z

@wgu-ram-chandra Please review this PR.

…FILES_STORAGE Depriciate STATICFILES_STORAGE and DEFAULT_FILE_STORAGE, and use STORAGE in Django 5.2

deborahgu · 2025-09-02T17:19:03Z

this is awaiting review from @wgu-ram-chandra, but also, I assume that this one should not be merged until all of the pending requests have been resolved on the related PR openedx/edx-platform#37002, because any related issues should presumably be addressed here as well.

awais786 · 2025-09-02T18:22:09Z

@deborahgu Yes we are following that as well

wgu-ram-chandra · 2025-09-02T21:29:34Z

Hi @awais786 @deborahgu, sorry for the delay. I'll review it today!

wgu-ram-chandra

@mubbsharanwar Thanks for the PR! I’ve left a few comments, once those are addressed, I’ll be happy to take another look.

credentials/settings/base.py

wgu-ram-chandra · 2025-09-03T16:47:04Z

credentials/settings/devstack.py

 EMAIL_FILE_PATH = "/tmp/credentials-emails"

-DEFAULT_FILE_STORAGE = os.environ.get("DEFAULT_FILE_STORAGE", "django.core.files.storage.FileSystemStorage")
+STORAGES["default"]["BACKEND"] = os.environ.get(


os.environ.get() expects an environment variable name, but the current code passes the backend value instead. This ends up looking for an env var like "django.core.files.storage.FileSystemStorage", which won’t exist, so it always falls back to the default and the override never takes effect.

We could assign django.core.files.storage.FileSystemStorage directly, or alternatively read from DEFAULT_FILE_STORAGE and assign that to maintain backward compatibility.

wgu-ram-chandra · 2025-09-03T16:48:26Z

credentials/settings/devstack.py

 MEDIA_URL = os.environ.get("MEDIA_URL", "/media/")

-STATICFILES_STORAGE = os.environ.get("STATICFILES_STORAGE", "django.contrib.staticfiles.storage.StaticFilesStorage")
+STORAGES["staticfiles"]["BACKEND"] = "django.contrib.staticfiles.storage.StaticFilesStorage"


Same here! We could assign django.contrib.staticfiles.storage.StaticFilesStorage directly, or alternatively read from STATICFILES_STORAGE and assign for backward compatibility.

wgu-ram-chandra · 2025-09-03T17:03:05Z

credentials/settings/production.py


+    if "DEFAULT_FILE_STORAGE" in config_from_yaml:
+        STORAGES["default"] = {"BACKEND": config_from_yaml.pop("DEFAULT_FILE_STORAGE")}
+        vars().update(STORAGES)


Using vars().update(STORAGES) can be risky because it flattens nested dictionaries into module-level variables like default and staticfiles, which is undesirable. It’s preferable to update the STORAGES dictionary directly rather than modifying vars().

@mubbsharanwar it looks like this is still unaddressed.

wgu-ram-chandra · 2025-09-03T17:12:39Z

credentials/settings/production.py

+
    vars().update(config_from_yaml)

+    if "DEFAULT_FILE_STORAGE" in FILE_STORAGE_BACKEND:


At this point, it’s not obvious what structure FILE_STORAGE_BACKEND takes, it’s initialized as {} and later populated from CREDENTIALS_CFG. The if "DEFAULT_FILE_STORAGE" logic seems to assume it’s a flat dict with backend paths as values. Could you confirm whether that assumption is correct?

Yes, that assumption is correct. DEFAULT_FILE_STORAGE is a flat dict.

wgu-ram-chandra · 2025-09-03T17:13:42Z

credentials/settings/test.py

 # Local Directories
 TEST_ROOT = path("test_root")
-DEFAULT_FILE_STORAGE = "django.core.files.storage.FileSystemStorage"
+STORAGES["default"]["BACKEND"] = os.environ.get(


Same as noted above in credentials/settings/devstack.py, this also applies to the old keys here.

kdmccormick · 2025-09-10T20:01:28Z

Could you get review from a maintainer instead?

awais786 · 2025-09-11T06:48:31Z

@deborahgu Please review this PR.

awais786 · 2025-09-19T09:09:23Z

@feanil This is final PR with default django52 for credentials. We will merge after 2844.

awais786 · 2025-09-19T15:21:22Z

@deborahgu Please wait let me verify one thing.

awais786 · 2025-09-22T11:31:35Z

@deborahgu I just verified using this yaml

FILE_STORAGE_BACKEND:
  AWS_S3_OBJECT_PARAMETERS:
     CacheControl: max-age=31536000
  AWS_QUERYSTRING_AUTH: false
  AWS_QUERYSTRING_EXPIRE: false
  AWS_S3_CUSTOM_DOMAIN: stage.edx-cdn.org
  AWS_STORAGE_BUCKET_NAME: stage-edx
  DEFAULT_FILE_STORAGE: storages.backends.s3boto3.S3Boto3Storage
  MEDIA_ROOT: media
  MEDIA_URL: https://stage.edx-cdn.org/media/

So the django shell showing following output

>>> from django.conf import settings
>>> settings.STORAGES
{
'default': {'BACKEND': 'django.core.files.storage.FileSystemStorage'}, 
'staticfiles': {'BACKEND': 'django.contrib.staticfiles.storage.ManifestStaticFilesStorage'}
}
>>> settings.AWS_STORAGE_BUCKET_NAME
'stage-edx'
>>> settings.FILE_STORAGE_BACKEND
{'AWS_S3_OBJECT_PARAMETERS': 
{'CacheControl': 'max-age=31536000'}, 'AWS_QUERYSTRING_AUTH': False,
 'AWS_QUERYSTRING_EXPIRE': False, 
'AWS_S3_CUSTOM_DOMAIN': 'stage.edx-cdn.org',
 'AWS_STORAGE_BUCKET_NAME': 'stage-edx', 'MEDIA_ROOT': 'media',
 'MEDIA_URL': 'https://stage.edx-cdn.org/media/', 
'STORAGES': {'default': {'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage'}}}

Observation:

FILE_STORAGE_BACKEND contains the correct S3 configuration.

AWS_STORAGE_BUCKET_NAME is correctly set.

wrong behaviour
However, settings.STORAGES is still pointing to the default local file system instead of S3.

This is the root cause.

FILE_STORAGE_BACKEND.update(
      {
          "STORAGES": {
              "default": {"BACKEND": default_file_storage},
          }
      }
  )

awais786 · 2025-09-22T12:11:20Z

I don't have the domain knowledge but you can comment or test this PR as it is. If purpose is only to set FILE_STORAGE_BACKEND then its fine.

feanil · 2025-09-05T14:09:38Z

credentials/settings/production.py


+    if "DEFAULT_FILE_STORAGE" in config_from_yaml:
+        STORAGES["default"] = {"BACKEND": config_from_yaml.pop("DEFAULT_FILE_STORAGE")}
+        vars().update(STORAGES)


@mubbsharanwar it looks like this is still unaddressed.

feanil · 2025-09-22T17:12:48Z

credentials/settings/production.py

-    vars().update(FILE_STORAGE_BACKEND)
+    if "DEFAULT_FILE_STORAGE" in FILE_STORAGE_BACKEND:
+        default_file_storage = FILE_STORAGE_BACKEND.pop("DEFAULT_FILE_STORAGE")
+        vars().update(FILE_STORAGE_BACKEND)


@mubbsharanwar I think the bug is here. By running this vars().update(FILE_STORAGE_BACKEND) won't this will take the keys and values inside that dict and drop it at the top level of the settings dict? Is that the intention?

@deborahgu it looks like FILE_STORAGE_BACKEND is not a django notion but something we may have invented in our settings. Is this being used in credentials settings? How is it structured? I don't see any references to it in the code.

@feanil I’ll fix it now; I just needed some clarification on the expected behavior.

@feanil I think I got the issue. I will fix both the PRs in credentials and disco.

Sample yaml

FILE_STORAGE_BACKEND AWS_STORAGE_BUCKET_NAME: stage-edx-catalog DEFAULT_FILE_STORAGE: storages.backends.s3boto3.S3Boto3Storage1234

All the keys from FILE_STORAGE_BACKEND are promoted to the root-level Django settings.
As soon as DEFAULT_FILE_STORAGE is set, Django automatically updates the STORAGES dict to reflect it.

IN django==4.2.24 you see this.

>>> from django.conf import settings >>> settings.STORAGES {'default': {'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage1234'}, 'staticfiles': {'BACKEND': 'django.contrib.staticfiles.storage.StaticFilesStorage'}} >>> settings.DEFAULT_FILE_STORAGE 'storages.backends.s3boto3.S3Boto3Storage1234'

So, setting DEFAULT_FILE_STORAGE directly ensures both DEFAULT_FILE_STORAGE and STORAGES['default'] stay in sync.

But for django52 we need to do this explicitly. So code is working due the root values set in production.

feat!: fixing storages.

feat!: fixing inline comment.

awais786 · 2025-09-23T13:38:56Z

@feanil @deborahgu Please review now.

hunzlahmalik · 2025-09-23T13:34:39Z

credentials/settings/base.py

+    "STORAGES": {
+        "default": {"BACKEND": STORAGES["default"]["BACKEND"]},
+    },


Suggested change

"STORAGES": {

"default": {"BACKEND": STORAGES["default"]["BACKEND"]},

},

"STORAGES": STORAGES

hunzlahmalik · 2025-09-23T13:38:04Z

credentials/settings/devstack.py

+STORAGES = {
+    "default": {
+        "BACKEND": "django.core.files.storage.FileSystemStorage",
+    },
+    "staticfiles": {
+        "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
+    },
+}


No need to redefine STORAGES, its coming from base.py

hunzlahmalik · 2025-09-23T13:39:04Z

credentials/settings/devstack.py

 EMAIL_BACKEND = "django.core.mail.backends.filebased.EmailBackend"
 EMAIL_FILE_PATH = "/tmp/credentials-emails"

-DEFAULT_FILE_STORAGE = os.environ.get("DEFAULT_FILE_STORAGE", "django.core.files.storage.FileSystemStorage")


defaultfile_storage = os.environ.get("DEFAULT_FILE_STORAGE", "django.core.files.storage.FileSystemStorage") if defaultfile_storage: STORAGES["default"]["BACKEND"] = defaultfile_storage

hunzlahmalik · 2025-09-23T13:39:38Z

credentials/settings/devstack.py

+    },
+}

-STATICFILES_STORAGE = os.environ.get("STATICFILES_STORAGE", "django.contrib.staticfiles.storage.StaticFilesStorage")


staticfiles_storage = os.environ.get('STATICFILES_STORAGE', 'django.contrib.staticfiles.storage.StaticFilesStorage') if staticfiles_storage: STORAGES["staticfiles"]["BACKEND"] = staticfiles_storage

hunzlahmalik · 2025-09-23T13:41:26Z

credentials/settings/test.py

+
+STORAGES = {
+    "default": {
+        "BACKEND": "django.core.files.storage.FileSystemStorage",
+    },
+    "staticfiles": {
+        "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
+    },
+}
+


No need to redefine the STORAGES

feat!: fixing inline comment.

awais786 · 2025-09-24T14:16:36Z

@feanil its ready now.

feanil · 2025-09-26T20:49:54Z

Looks good, I'll do a final review and merge on Monday.

awais786 · 2025-09-29T09:57:03Z

For tracking purpose.

openedx/public-engineering#419

credentials/settings/devstack.py

Don't set defaults when that are the same as the defaults from base.py setting them in multiple places could lead to confusion and bugs in the future.

github-project-automation bot added this to Aperture-Maintained Aug 25, 2025

mubbsharanwar force-pushed the mubbsharanwar/upgrade-storages branch 2 times, most recently from bfc5722 to 029594a Compare August 25, 2025 09:47

mubbsharanwar requested a review from awais786 August 25, 2025 13:41

awais786 mentioned this pull request Aug 26, 2025

django52: remove deprecated DEFAULT_FILE_STORAGE and STATICFILES_STORAGE #2828

Closed

3 tasks

mubbsharanwar force-pushed the mubbsharanwar/upgrade-storages branch 6 times, most recently from d14cbac to 7cd09bf Compare August 26, 2025 14:45

mubbsharanwar force-pushed the mubbsharanwar/upgrade-storages branch 3 times, most recently from 9733674 to 10baf35 Compare August 27, 2025 08:50

mubbsharanwar force-pushed the mubbsharanwar/upgrade-storages branch from 10baf35 to 29560aa Compare September 2, 2025 06:06

fix: Django52 use STORAGES instead of DEFAULT_FILE_STORAGE and STATIC…

7ab9ce3

…FILES_STORAGE Depriciate STATICFILES_STORAGE and DEFAULT_FILE_STORAGE, and use STORAGE in Django 5.2

mubbsharanwar force-pushed the mubbsharanwar/upgrade-storages branch from 29560aa to 7ab9ce3 Compare September 2, 2025 06:10

mubbsharanwar changed the title ~~chore: use Django 5.2 STORAGES~~ fix: Django52 use STORAGES instead of DEFAULT_FILE_STORAGE and STATICFILES_STORAGE Sep 2, 2025

deborahgu moved this to Todo in Aperture-Maintained Sep 2, 2025

deborahgu added the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Sep 2, 2025

deborahgu requested a review from wgu-ram-chandra September 2, 2025 17:17

wgu-ram-chandra suggested changes Sep 3, 2025

View reviewed changes

mubbsharanwar force-pushed the mubbsharanwar/upgrade-storages branch 2 times, most recently from c919ce7 to 54e6d0f Compare September 4, 2025 06:57

feanil requested review from a team September 12, 2025 14:38

deborahgu approved these changes Sep 18, 2025

View reviewed changes

feanil reviewed Sep 22, 2025

View reviewed changes

awais786 added 6 commits September 23, 2025 18:16

feat!: fixing storages.

475f37b

Merge pull request #1 from awais786/upgrade-storages

e403c34

feat!: fixing storages.

feat!: fixing storages.

3d3bd4d

Merge pull request #2 from awais786/upgrade-storages

98dd8e0

feat!: fixing storages.

feat!: fixing inline comment.

4328721

Merge pull request #3 from awais786/upgrade-storages

4743ef3

feat!: fixing inline comment.

hunzlahmalik reviewed Sep 23, 2025

View reviewed changes

awais786 added 4 commits September 23, 2025 19:13

feat!: fixing inline comment.

1f49a22

Merge pull request #4 from awais786/upgrade-storages

ef8a1e9

feat!: fixing inline comment.

feat!: fixing inline comment.

36022e0

Merge pull request #5 from awais786/upgrade-storages

dd3301a

feat!: fixing inline comment.

awais786 mentioned this pull request Sep 26, 2025

feat!: upgrading to django52. #2861

Merged

feanil reviewed Sep 29, 2025

View reviewed changes

credentials/settings/devstack.py Outdated Show resolved Hide resolved

credentials/settings/devstack.py Outdated Show resolved Hide resolved

fix: Apply suggestions from code review

1408832

Don't set defaults when that are the same as the defaults from base.py setting them in multiple places could lead to confusion and bugs in the future.

feanil merged commit 53d1a60 into openedx:master Sep 29, 2025
10 checks passed

github-project-automation bot moved this from Todo to Done in Aperture-Maintained Sep 29, 2025


		vars().update(config_from_yaml)

		if "DEFAULT_FILE_STORAGE" in FILE_STORAGE_BACKEND:

fix: Django52 use STORAGES instead of DEFAULT_FILE_STORAGE and STATICFILES_STORAGE #2844

fix: Django52 use STORAGES instead of DEFAULT_FILE_STORAGE and STATICFILES_STORAGE #2844

Uh oh!

Conversation

mubbsharanwar commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

deborahgu commented Aug 25, 2025

Uh oh!

awais786 commented Aug 26, 2025

Uh oh!

awais786 commented Aug 27, 2025

Uh oh!

deborahgu commented Sep 2, 2025

Uh oh!

awais786 commented Sep 2, 2025

Uh oh!

wgu-ram-chandra commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wgu-ram-chandra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kdmccormick commented Sep 10, 2025

Uh oh!

awais786 commented Sep 11, 2025

Uh oh!

awais786 commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awais786 commented Sep 19, 2025

Uh oh!

awais786 commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

awais786 commented Sep 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awais786 Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awais786 commented Sep 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

awais786 commented Sep 24, 2025

Uh oh!

mubbsharanwar commented Aug 25, 2025 •

edited

Loading

wgu-ram-chandra commented Sep 2, 2025 •

edited

Loading

awais786 commented Sep 19, 2025 •

edited

Loading

awais786 commented Sep 22, 2025 •

edited

Loading

awais786 commented Sep 22, 2025 •

edited

Loading

awais786 Sep 23, 2025 •

edited

Loading

awais786 commented Sep 29, 2025 •

edited

Loading