Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions credentials/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,14 @@
FILE_STORAGE_BACKEND = {}
EXTRA_APPS = []
SESSION_EXPIRE_AT_BROWSER_CLOSE = False
STATICFILES_STORAGE = "django.contrib.staticfiles.storage.ManifestStaticFilesStorage"
STORAGES = {
"default": {
"BACKEND": "django.core.files.storage.FileSystemStorage",
},
"staticfiles": {
"BACKEND": "django.contrib.staticfiles.storage.ManifestStaticFilesStorage",
},
}
CACHES = {
"default": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
Expand All @@ -474,7 +481,9 @@
}
API_ROOT = None
MEDIA_STORAGE_BACKEND = {
"DEFAULT_FILE_STORAGE": "django.core.files.storage.FileSystemStorage",
"STORAGES": {
"default": {"BACKEND": STORAGES["default"]["BACKEND"]},
},

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"MEDIA_ROOT": MEDIA_ROOT,
"MEDIA_URL": MEDIA_URL,
}
Expand Down
12 changes: 9 additions & 3 deletions credentials/settings/devstack.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,16 @@
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")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if defaultfile_storage:
    STORAGES["default"]["BACKEND"] = defaultfile_storage

MEDIA_URL = os.environ.get("MEDIA_URL", "/media/")
STORAGES = {
"default": {
"BACKEND": "django.core.files.storage.FileSystemStorage",
},
"staticfiles": {
"BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
},
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

staticfiles_storage = os.environ.get('STATICFILES_STORAGE', 'django.contrib.staticfiles.storage.StaticFilesStorage')

if staticfiles_storage:
    STORAGES["staticfiles"]["BACKEND"] = staticfiles_storage

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

# OAuth2 variables specific to social-auth/SSO login use case.
Expand Down
24 changes: 23 additions & 1 deletion credentials/settings/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,32 @@
if value:
vars()[key].update(value)

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

if "STATICFILES_STORAGE" in config_from_yaml:
STORAGES["staticfiles"] = {
"BACKEND": config_from_yaml.pop("STATICFILES_STORAGE"),
}

vars().update(config_from_yaml)

# Load the files storage backend settings for django storages
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@awais786 awais786 Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


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

# make sure this happens after the configuration file overrides so format string can be overridden
LOGGING = get_logger_config(format_string=LOGGING_FORMAT_STRING)
Expand Down
12 changes: 10 additions & 2 deletions credentials/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,16 @@

# Local Directories
TEST_ROOT = path("test_root")
DEFAULT_FILE_STORAGE = "django.core.files.storage.FileSystemStorage"

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to redefine the STORAGES

MEDIA_ROOT = str(TEST_ROOT / "uploads")
MEDIA_URL = "/static/uploads/"

Expand All @@ -49,7 +58,6 @@
"JWT_AUDIENCE": SOCIAL_AUTH_EDX_OAUTH2_KEY,
}
)
STATICFILES_STORAGE = "django.contrib.staticfiles.storage.StaticFilesStorage"

# Verifiable Credentials
ENABLE_VERIFIABLE_CREDENTIALS = True
Expand Down
60 changes: 60 additions & 0 deletions credentials/tests/test_production.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import builtins
import importlib
import io
import sys
import types


def test_production_storage_from_yaml(monkeypatch):
"""Test that YAML config correctly populates STORAGES without leaking globals."""

def fake_get_env_setting(key):
if key == "CREDENTIALS_CFG":
return "/fake/path/config.yaml"
return ""

class FakeSettingsType:
PRODUCTION = "production"

fake_yaml_content = """
DEFAULT_FILE_STORAGE: storages.backends.s3boto3.S3Boto3Storage
STATICFILES_STORAGE: storage.ManifestStaticFilesStorage
MEDIA_ROOT: /tmp/media
MEDIA_URL: /media/
"""

sys.modules.pop("credentials.settings.production", None)

monkeypatch.setitem(
sys.modules,
"credentials.settings.utils",
types.SimpleNamespace(
get_env_setting=fake_get_env_setting,
get_logger_config=lambda *a, **kw: {},
),
)
monkeypatch.setitem(
sys.modules,
"credentials.apps.plugins.constants",
types.SimpleNamespace(
PROJECT_TYPE="credentials.djangoapp",
SettingsType=FakeSettingsType,
),
)
monkeypatch.setitem(
sys.modules,
"edx_django_utils.plugins",
types.SimpleNamespace(add_plugins=lambda *a, **kw: None),
)

monkeypatch.setattr(builtins, "open", lambda *a, **kw: io.StringIO(fake_yaml_content))

prod = importlib.import_module("credentials.settings.production")

assert not hasattr(prod, "DEFAULT_FILE_STORAGE")
assert not hasattr(prod, "STATICFILES_STORAGE")

assert "default" in prod.STORAGES
assert prod.STORAGES["default"]["BACKEND"] == "storages.backends.s3boto3.S3Boto3Storage"
assert "staticfiles" in prod.STORAGES
assert prod.STORAGES["staticfiles"]["BACKEND"] == "storage.ManifestStaticFilesStorage"