feat!: upgrade code and fix get_storage_class ( compatibility django42 and django52)

[awais786]

openedx/public-engineering#340

Fixing breaking change for django52.

• Verify the image functionality on sandbox.

[musanaeem]

Unsure if this case could ever occur but if the contents of storage_class_path are incorrect, there could be an error which we should catch e.g

try: storage_class = import_string(storage_class_path) except ImportError as e: raise ImproperlyConfigured( f"Could not import storage class '{storage_class_path}': {e}" )

[awais786]

https://github.com/carltongibson/django/blob/5b1099323288b15a4c23cba88eb4dcdfa824b3d4/django/core/files/storage/__init__.py#L37C5-L37C71

[musanaeem]

LGTM!

[hinakhadim]

Why are not we using the below one?

from django.core.files.storage import storages
storage_class = storages(config.get('class'))
return storage_class(**config.get('options', {}))

[awais786]

https://github.com/carltongibson/django/blob/5b1099323288b15a4c23cba88eb4dcdfa824b3d4/django/core/files/storage/__init__.py#L37C5-L37C71

[awais786]

@feanil Please review

[feanil]

Before we fallback to the default storage class, I think we should also have a named storage class in the STORAGES dictionary. Maybe named profile_images?

This way, operators could migrate to defining their various storages config via the new STORAGES config and we can eventually deprecate the old configuration settings.

[awais786]

@feanil you mean some thing like this

storage_class_path = (
    getattr(settings, 'DEFAULT_FILE_STORAGE', None)  # legacy setting
    or getattr(settings, 'STORAGES', {}).get('profile_images', {}).get('BACKEND')  # named storage
    or getattr(settings, 'STORAGES', {}).get('default', {}).get('BACKEND')  # default
    or 'django.core.files.storage.FileSystemStorage'  # final fallback
)

Along some settings in common.py

STORAGES = {
    'profile_images': {
        'BACKEND': 'storages.backends.s3boto3.S3Boto3Storage',
        'OPTIONS': {
            'bucket_name': 'my-bucket',
            ...
        }
    },
    'default': {
        'BACKEND': 'django.core.files.storage.FileSystemStorage'
    }
}

[awais786]

@feanil I have verified on django42 this solution will not work. These settings cannot declare together.

STORAGES = {
    "profile_images": {
        "BACKEND": "openedx.core.storage.OverwriteStorage",
        "OPTIONS": {
            "location": "profile-images/",
            "base_url": "profile-images/",
        },
    },
    'default': {
        'BACKEND': 'django.core.files.storage.FileSystemStorage'
    }
}

DEFAULT_FILE_STORAGE = 'django.core.files.storage.FileSystemStorage'

These cannot declared together. It raises error

django.core.exceptions.ImproperlyConfigured: DEFAULT_FILE_STORAGE/STORAGES are mutually exclusive.

Django code reference

Right now DEFAULT_FILE_STORAGE is declared at multiple places and this change will increase the scope of this PR. We can add followup ticket for cleaning purpose. openedx/public-engineering#369

[awais786]

profile_images it does not exists right now so it will use following storages.