diff --git a/readthedocs/projects/tasks/storage.py b/readthedocs/projects/tasks/storage.py index 4613cb71aba..9a6bc386676 100644 --- a/readthedocs/projects/tasks/storage.py +++ b/readthedocs/projects/tasks/storage.py @@ -3,7 +3,7 @@ import structlog from django.conf import settings -from django.utils.module_loading import import_string +from django.core.files.storage import storages from readthedocs.doc_builder.exceptions import BuildAppError from readthedocs.projects.models import Feature @@ -53,13 +53,24 @@ def _get_storage_class(storage_type: StorageType): raise ValueError("Invalid storage type") +def _get_storage_backend_class(alias: str): + """ + Get the storage backend class for a given alias from STORAGES setting. + + This returns the class, not an instance, so it can be instantiated + with custom kwargs (e.g., per-build credentials). + """ + storage = storages[alias] + return type(storage) + + def _get_build_media_storage_class(): """ Get a storage class to use for syncing build artifacts. This is done in a separate method to make it easier to mock in tests. """ - return import_string(settings.RTD_BUILD_MEDIA_STORAGE) + return _get_storage_backend_class("build-media") def _get_build_tools_storage_class(): @@ -68,7 +79,7 @@ def _get_build_tools_storage_class(): This is done in a separate method to make it easier to mock in tests. """ - return import_string(settings.RTD_BUILD_TOOLS_STORAGE) + return _get_storage_backend_class("build-tools") def _get_s3_scoped_credentials(*, project, build_id, api_client, storage_type: StorageType): diff --git a/readthedocs/proxito/tests/base.py b/readthedocs/proxito/tests/base.py index 869a54fe994..e4dc231a118 100644 --- a/readthedocs/proxito/tests/base.py +++ b/readthedocs/proxito/tests/base.py @@ -2,9 +2,8 @@ import django_dynamic_fixture as fixture import pytest -from django.conf import settings from django.contrib.auth.models import User -from readthedocs.storage import get_storage_class +from django.core.files.storage import storages from django.test import TestCase from readthedocs.builds.constants import LATEST @@ -19,9 +18,7 @@ def setUp(self): # Re-initialize storage # Various tests override either this setting or various aspects of the storage engine # By resetting it every test case, we avoid this caching (which is a huge benefit in prod) - serve.build_media_storage = get_storage_class( - settings.RTD_BUILD_MEDIA_STORAGE - )() + serve.build_media_storage = storages["build-media"] self.eric = fixture.get(User, username="eric") self.eric.set_password("eric") diff --git a/readthedocs/rtd_tests/tests/test_imported_file.py b/readthedocs/rtd_tests/tests/test_imported_file.py index d5d901225e6..4f0f0a85701 100644 --- a/readthedocs/rtd_tests/tests/test_imported_file.py +++ b/readthedocs/rtd_tests/tests/test_imported_file.py @@ -2,8 +2,7 @@ from unittest import mock import pytest -from django.conf import settings -from readthedocs.storage import get_storage_class +from django.core.files.storage import storages from django.test import TestCase from django.test.utils import override_settings from django_dynamic_fixture import get @@ -21,7 +20,9 @@ @pytest.mark.search @override_settings(ELASTICSEARCH_DSL_AUTOSYNC=True) class ImportedFileTests(TestCase): - storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)() + @property + def storage(self): + return storages["build-media"] def setUp(self): self.project = get(Project) diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index 2c8eba8f030..26f0bc24925 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -1192,16 +1192,29 @@ def GITHUB_APP_CLIENT_ID(self): @property def STORAGES(self): # https://django-storages.readthedocs.io/en/latest/backends/amazon-S3.html + # https://docs.djangoproject.com/en/5.2/ref/settings/#std-setting-STORAGES return { + "default": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + }, "staticfiles": { - "BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage" + "BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage", + }, + "build-media": { + "BACKEND": self.RTD_BUILD_MEDIA_STORAGE, + }, + "build-commands": { + "BACKEND": self.RTD_BUILD_COMMANDS_STORAGE, + }, + "build-tools": { + "BACKEND": self.RTD_BUILD_TOOLS_STORAGE, }, "usercontent": { "BACKEND": "django.core.files.storage.FileSystemStorage", "OPTIONS": { "location": Path(self.MEDIA_ROOT) / "usercontent", "allow_overwrite": True, - } + }, }, } diff --git a/readthedocs/settings/docker_compose.py b/readthedocs/settings/docker_compose.py index 105ed8decef..269d635c7dd 100644 --- a/readthedocs/settings/docker_compose.py +++ b/readthedocs/settings/docker_compose.py @@ -243,8 +243,20 @@ def SOCIALACCOUNT_PROVIDERS(self): @property def STORAGES(self): return { + "default": { + "BACKEND": "django.core.files.storage.FileSystemStorage", + }, "staticfiles": { - "BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage" + "BACKEND": "readthedocs.storage.s3_storage.S3StaticStorage", + }, + "build-media": { + "BACKEND": self.RTD_BUILD_MEDIA_STORAGE, + }, + "build-commands": { + "BACKEND": self.RTD_BUILD_COMMANDS_STORAGE, + }, + "build-tools": { + "BACKEND": self.RTD_BUILD_TOOLS_STORAGE, }, "usercontent": { "BACKEND": "storages.backends.s3.S3Storage", diff --git a/readthedocs/settings/test.py b/readthedocs/settings/test.py index 7713667db33..501ce697367 100644 --- a/readthedocs/settings/test.py +++ b/readthedocs/settings/test.py @@ -142,6 +142,15 @@ def STORAGES(self): "staticfiles": { "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage", }, + "build-media": { + "BACKEND": self.RTD_BUILD_MEDIA_STORAGE, + }, + "build-commands": { + "BACKEND": self.RTD_BUILD_COMMANDS_STORAGE, + }, + "build-tools": { + "BACKEND": self.RTD_BUILD_TOOLS_STORAGE, + }, "usercontent": { "BACKEND": "django.core.files.storage.FileSystemStorage", "OPTIONS": { diff --git a/readthedocs/storage/__init__.py b/readthedocs/storage/__init__.py index 1f8981c7d51..917b9afe316 100644 --- a/readthedocs/storage/__init__.py +++ b/readthedocs/storage/__init__.py @@ -6,38 +6,28 @@ so doing those upfront improves performance. """ -from django.conf import settings +from django.core.files.storage import storages from django.utils.functional import LazyObject -from django.utils.module_loading import import_string - - -# Borrowed from Django 4.2 since it was deprecrated and removed in 5.2 -# NOTE: we can use settings.STORAGES for our own storages as well if we want to use the standards. -# -# https://docs.djangoproject.com/en/5.0/ref/settings/#std-setting-STORAGES) -# https://github.com/django/django/blob/4.2/django/core/files/storage/__init__.py#L31 -def get_storage_class(import_path=None): - return import_string(import_path or settings.DEFAULT_FILE_STORAGE) class ConfiguredBuildMediaStorage(LazyObject): def _setup(self): - self._wrapped = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)() + self._wrapped = storages["build-media"] class ConfiguredBuildCommandsStorage(LazyObject): def _setup(self): - self._wrapped = get_storage_class(settings.RTD_BUILD_COMMANDS_STORAGE)() + self._wrapped = storages["build-commands"] class ConfiguredBuildToolsStorage(LazyObject): def _setup(self): - self._wrapped = get_storage_class(settings.RTD_BUILD_TOOLS_STORAGE)() + self._wrapped = storages["build-tools"] class ConfiguredStaticStorage(LazyObject): def _setup(self): - self._wrapped = get_storage_class(settings.RTD_STATICFILES_STORAGE)() + self._wrapped = storages["staticfiles"] build_media_storage = ConfiguredBuildMediaStorage()