fix: resolve feedback comments

This commit is contained in:
Daniel Wong
2025-05-21 13:38:30 -06:00
parent 835b74bb4f
commit 88ee5b4d28
2 changed files with 24 additions and 25 deletions

View File

@@ -78,6 +78,7 @@ class TestExportCourseMetadata(SharedModuleStoreTestCase):
self.assertEqual(storage.__class__.__name__, "CourseMetadataExportS3Storage")
self.assertEqual(storage.bucket_name, "bucket_name_test")
@override_settings()
def test_resolve_storage_with_no_config(self):
""" If no storage setup is defined, we get FileSystemStorage by default """
del settings.DEFAULT_FILE_STORAGE
@@ -114,7 +115,6 @@ class TestExportCourseMetadata(SharedModuleStoreTestCase):
)
def test_resolve_storage_using_django5_settings_with_options(self):
""" Ensure we call the storage class with the correct parameters and Django 5 setup """
del settings.DEFAULT_FILE_STORAGE
del settings.COURSE_METADATA_EXPORT_STORAGE
del settings.COURSE_METADATA_EXPORT_BUCKET
storage = resolve_storage_backend("COURSE_METADATA_EXPORT_STORAGE")

View File

@@ -1,24 +1,28 @@
""" Utility functions related to django storages """
import django
from typing import Optional
from django.conf import settings
from django.core.files.storage import default_storage
from django.core.files.storage import default_storage, storages
from django.utils.module_loading import import_string
def resolve_storage_backend(storage_key, options=None):
def resolve_storage_backend(storage_key: str, options: Optional[dict] = None):
"""
Configures and returns a Django `Storage` instance, compatible with both Django 4 and Django 5.
Main goal:
Deprecate the use of `django.core.files.storage.get_storage_class`.
How:
Replace `get_storage_class` with direct configuration logic,
ensuring backward compatibility with both Django 4 and Django 5 storage settings.
Params:
storage_key = The key name saved in Django settings.
options = Kwargs for the storage class
Returns:
An instance of the configured storage backend.
Raises:
ImportError: If the specified storage class cannot be imported.
Main goal:
Deprecate the use of `django.core.files.storage.get_storage_class`.
How:
Replace `get_storage_class` with direct configuration logic,
ensuring backward compatibility with both Django 4 and Django 5 storage settings.
"""
storage_path = getattr(settings, storage_key, None)
@@ -29,33 +33,28 @@ def resolve_storage_backend(storage_key, options=None):
if storage_key == "default":
# Use case 1: Default storage
# Works consistently across Django 4.2 and 5.x
# In Django 4.2, default_storage uses DEFAULT_FILE_STORAGE
# In Django 5.x, it uses STORAGES['default']
# Works consistently across Django 4.2 and 5.x.
# In Django 4.2 and above, `default_storage` uses
# either `DEFAULT_FILE_STORAGE` or `STORAGES['default']`.
return default_storage
if django.VERSION >= (5, 0) and storage_key in storages_config:
# Use case 2: Django 5+ with STORAGES defined
if storage_key in storages_config:
# Use case 2: STORAGES is defined
# If STORAGES is present, we retrieve it through the storages API
# settings.py must define STORAGES like:
# STORAGES = {
# "default": {"BACKEND": "...", "OPTIONS": {...}},
# "custom": {"BACKEND": "...", "OPTIONS": {...}},
# }
# See: https://docs.djangoproject.com/en/5.2/ref/settings/#std-setting-STORAGES
from django.core.files.storage import storages
return storages[storage_key]
if not storage_path and storage_key in storages_config:
# Use case 3: Transitional setup
# Running on Django 4.x but using Django 5-style STORAGES config
# Manually load the backend and options
storage_path = storages_config.get(storage_key, {}).get("BACKEND")
options = storages_config.get(storage_key, {}).get("OPTIONS", {})
if not storage_path:
# if no specific storage was resolved, use the default storage
# Use case 3: No storage settings defined
# If no storage settings are defined anywhere, use the default storage
return default_storage
# Fallback to import the storage_path manually
# Use case 4: Import storage_path manually
# Fallback to import the storage_path (Obtained from django settings) manually
StorageClass = import_string(storage_path)
return StorageClass(**options)