Upgrade STORAGES setting to use Django's standard pattern #12625

Copilot · 2025-12-01T11:21:07Z

Migrates custom storage backends (build-media, build-commands, build-tools) to Django's STORAGES setting, enabling use of the standard django.core.files.storage.storages API.

Changes

Settings: Added storage aliases to STORAGES property in base.py, docker_compose.py, and test.py
Storage module: Updated readthedocs/storage/__init__.py to use storages["alias"] instead of get_storage_class(settings.RTD_*)
Build tasks: Updated readthedocs/projects/tasks/storage.py to retrieve storage classes via storages API
Test files: Migrated proxito/tests/base.py and rtd_tests/tests/test_imported_file.py to new pattern
Deprecation: Added deprecation warning to get_storage_class() function

Usage

# Before
from readthedocs.storage import get_storage_class
storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()

# After
from django.core.files.storage import storages
storage = storages["build-media"]

The lazy-loaded instances (build_media_storage, build_commands_storage, etc.) continue to work unchanged.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

search
- Triggering command: /usr/bin/python python -m pytest readthedocs/rtd_tests/tests/test_imported_file.py -v --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

This section details on the original issue you should resolve

<issue_title>Upgrade STORAGES setting to the new pattern</issue_title>
<issue_description>This is a follow up of the changes introduced in:

Upload staticfiles to S3 #12220

We upgrade our settings to be like:
    @property
    def STORAGES(self):
        return {
            "staticfiles": {
                "BACKEND": self.RTD_STATICFILES_STORAGE,
                "OPTIONS": {
                    "bucket_acl": "public-read",
                    "default_acl": "public-read",
                    "querystring_auth": False,
                },
            },

            "build-media": {
                "BACKEND": self.RTD_BUILD_MEDIA_STORAGE,
                "OPTIONS": {
                    "bucket_name": self.S3_MEDIA_STORAGE_BUCKET,
                    "custom_domain": self.S3_MEDIA_STORAGE_OVERRIDE_HOSTNAME,
                    "bucket_acl": "public-read",
                    "default_acl": "public-read",
                    "querystring_auth": False,
                },
            },

            # ... other storage backends here like `build-tools` and more ...
        }
With this, we will be able to delete all our classes and mixins to deal with multiple storages and make the code cleaner while using the Django standards:

https://github.com/readthedocs/readthedocs.org/blob/humitos%2Fstaticfiles/readthedocs/storage/__init__.py#L43-L46

https://github.com/readthedocs/readthedocs.org/blob/humitos%2Fstaticfiles/readthedocs/storage/s3_storage.py#L26-L121</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes Upgrade STORAGES setting to the new pattern #12221

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: humitos <244656+humitos@users.noreply.github.com>

humitos

Looks good to me. I tested it locally and it worked. I will check -ops repositories and corporate now.

readthedocs/storage/__init__.py

humitos · 2025-12-02T10:07:38Z

I will check -ops repositories and corporate now.

I didn't find any code that requires changes there.

readthedocs/projects/tasks/storage.py

stsewd · 2025-12-03T01:11:24Z

readthedocs/projects/tasks/storage.py

+    storage = storages[alias]
+    return type(storage)


Looks like this can be done with

import_string(storages.backends[alias]['BACKEND'])

since there is no need for an instance

Yeah, we could use import_string but since the class is already imported, calling type over the instance makes more sense to me than re-importing.

stsewd · 2025-12-03T01:12:52Z

readthedocs/proxito/tests/base.py

-        serve.build_media_storage = get_storage_class(
-            settings.RTD_BUILD_MEDIA_STORAGE
-        )()
+        serve.build_media_storage = storages["build-media"]


Checking the comment, this instance is also probably cached and will make some tests fail.

It seems that all tests are passing.

Should we remove this line completely? Seems Django is smart enough and it's not required anymore?

Suggested change

serve.build_media_storage = storages["build-media"]

readthedocs/storage/__init__.py

stsewd · 2025-12-03T01:17:43Z

readthedocs/storage/__init__.py

 class ConfiguredBuildMediaStorage(LazyObject):
    def _setup(self):
-        self._wrapped = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
+        self._wrapped = storages["build-media"]


I think these were made in order to lazy instantiate the storage object. storages[alias] does that by default, probably? We should check, we don't need these wrappers otherwise.

Hrm, I'm not sure. The documentation doesn't say too much about lazyness.

I asked AI about this and it says that storage[alias] is not lazy by default and we should do what we are currently doing if we want it to be lazy.

I checked the Django's code and it seems it's not lazy: https://github.com/django/django/blob/dba622ebc1f6a6700b75303a65f8a334bd46bd8e/django/core/files/storage/handler.py#L11

readthedocs/projects/tasks/storage.py

Co-authored-by: Santos Gallegos <stsewd@proton.me>

read-the-docs-community · 2025-12-03T11:40:30Z

Documentation build overview

📚 dev | 🛠️ Build #30549943 | 📁 Comparing 247f637 against latest (fd3bcfd)

🔍 Preview build

Show files changed (25 files in total): 📝 1 modified | ➕ 24 added | ➖ 0 deleted

File	Status
http-routingtable.html	➕ added
index.html	📝 modified
design/apiv3.html	➕ added
design/better-doc-urls-handling.html	➕ added
design/build-images.html	➕ added
design/embed-api.html	➕ added
design/file-tree-diff.html	➕ added
design/flyout-redesign.html	➕ added
design/future-builder.html	➕ added
design/in-doc-search-ui.html	➕ added
design/index.html	➕ added
design/new-notifications-system.html	➕ added
design/new-search-api.html	➕ added
design/new-sphinx-guides.html	➕ added
design/organizations.html	➕ added
design/pr-builder.html	➕ added
design/privacy-levels.html	➕ added
design/redirects.html	➕ added
design/refactor-remote-repository.html	➕ added
design/secure-api-access-from-builders.html	➕ added
design/sphinx-jquery.html	➕ added
design/system-packages.html	➕ added
design/telemetry.html	➕ added
design/theme-context.html	➕ added
design/yaml-file.html	➕ added

readthedocs/settings/base.py

humitos · 2025-12-09T17:45:18Z

@stsewd this is ready to re-review and merge from my side.

Initial plan

ada1ef5

Copilot AI assigned Copilot and humitos Dec 1, 2025

Copilot started work on behalf of humitos December 1, 2025 11:21 View session

Copilot AI and others added 2 commits December 1, 2025 11:48

Upgrade STORAGES setting to include custom storage backends

1fcb668

Co-authored-by: humitos <244656+humitos@users.noreply.github.com>

Address code review feedback: update test files to use storages API

90de92a

Co-authored-by: humitos <244656+humitos@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Upgrade STORAGES setting to the new pattern~~ Upgrade STORAGES setting to use Django's standard pattern Dec 1, 2025

Copilot AI requested a review from humitos December 1, 2025 11:59

Copilot finished work on behalf of humitos December 1, 2025 11:59

humitos approved these changes Dec 1, 2025

View reviewed changes

readthedocs/storage/__init__.py Outdated Show resolved Hide resolved

humitos marked this pull request as ready for review December 2, 2025 10:07

humitos requested a review from a team as a code owner December 2, 2025 10:07

humitos requested a review from stsewd December 2, 2025 10:07

stsewd reviewed Dec 3, 2025

View reviewed changes

humitos reviewed Dec 3, 2025

View reviewed changes

readthedocs/projects/tasks/storage.py Outdated Show resolved Hide resolved

humitos reviewed Dec 3, 2025

View reviewed changes

readthedocs/projects/tasks/storage.py Outdated Show resolved Hide resolved

Apply suggestions from code review

a848f3f

Co-authored-by: Santos Gallegos <stsewd@proton.me>

Remove old get_storage_class

bceb15d

humitos reviewed Dec 3, 2025

View reviewed changes

readthedocs/settings/base.py Outdated Show resolved Hide resolved

Apply suggestion from @humitos

247f637

humitos requested a review from stsewd December 3, 2025 12:24

Uh oh!

Upgrade STORAGES setting to use Django's standard pattern #12625

Are you sure you want to change the base?

Upgrade STORAGES setting to use Django's standard pattern #12625

Conversation

Copilot AI commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Usage

I tried to connect to the following addresses, but was blocked by firewall rules:

Comments on the Issue (you are @copilot in this section)

Uh oh!

humitos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

humitos commented Dec 2, 2025

Uh oh!

Uh oh!

stsewd Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

humitos Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

stsewd Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

humitos Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

humitos Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stsewd Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

humitos Dec 3, 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!

humitos Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

read-the-docs-community bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation build overview

Uh oh!

Uh oh!

humitos commented Dec 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Dec 1, 2025 •

edited

Loading

humitos Dec 3, 2025 •

edited

Loading

read-the-docs-community bot commented Dec 3, 2025 •

edited

Loading