Make storage classes into module level vars

Some storage backends (notably S3) have extra startup times
for the first connection to the backend (~50ms)
which means that the storage object has to be reused
for the best performance.
This reuses them by making our storage instances
into lazy loaded module level variables
exactly like Django does with `staticfiles_storage`.

Author: davidfischer

readthedocs/api/v2/views/model_views.py

@@ -5,7 +5,6 @@
 
 from allauth.socialaccount.models import SocialAccount
 from django.conf import settings
-from django.core.files.storage import get_storage_class
 from django.shortcuts import get_object_or_404
 from django.template.loader import render_to_string
 from rest_framework import decorators, permissions, status, viewsets
@@ -19,6 +18,7 @@
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
 from readthedocs.oauth.services import GitHubService, registry
 from readthedocs.projects.models import Domain, Project
+from readthedocs.storage import build_commands_storage
 
 from ..permissions import APIPermission, APIRestrictedPermission, IsOwner
 from ..serializers import (
@@ -257,14 +257,13 @@ def retrieve(self, *args, **kwargs):
         serializer = self.get_serializer(instance)
         data = serializer.data
         if instance.cold_storage:
-            storage = get_storage_class(settings.RTD_BUILD_COMMANDS_STORAGE)()
             storage_path = '{date}/{id}.json'.format(
                 date=str(instance.date.date()),
                 id=instance.id,
             )
-            if storage.exists(storage_path):
+            if build_commands_storage.exists(storage_path):
                 try:
-                    json_resp = storage.open(storage_path).read()
+                    json_resp = build_commands_storage.open(storage_path).read()
                     data['commands'] = json.loads(json_resp)
                 except Exception:
                     log.exception(

readthedocs/builds/models.py

@@ -8,7 +8,6 @@
 
 import regex
 from django.conf import settings
-from django.core.files.storage import get_storage_class
 from django.db import models
 from django.db.models import F
 from django.urls import reverse
@@ -88,6 +87,7 @@
 )
 from readthedocs.projects.models import APIProject, Project
 from readthedocs.projects.version_handling import determine_stable_version
+from readthedocs.storage import build_environment_storage
 
 log = logging.getLogger(__name__)
 
@@ -411,8 +411,7 @@ def get_storage_paths(self):
 
     def get_storage_environment_cache_path(self):
         """Return the path of the cached environment tar file."""
-        storage = get_storage_class(settings.RTD_BUILD_ENVIRONMENT_STORAGE)()
-        return storage.join(self.project.slug, f'{self.slug}.tar')
+        return build_environment_storage.join(self.project.slug, f'{self.slug}.tar')
 
     def clean_build_path(self):
         """

readthedocs/builds/tasks.py

@@ -5,7 +5,6 @@
 
 from celery import Task
 from django.conf import settings
-from django.core.files.storage import get_storage_class
 
 from readthedocs.api.v2.serializers import BuildSerializer
 from readthedocs.api.v2.utils import (
@@ -27,6 +26,7 @@
 from readthedocs.core.utils import trigger_build
 from readthedocs.projects.models import Project
 from readthedocs.projects.tasks import send_build_status
+from readthedocs.storage import build_commands_storage
 from readthedocs.worker import app
 
 log = logging.getLogger(__name__)
@@ -158,7 +158,6 @@ def archive_builds_task(days=14, limit=200, include_cold=False, delete=False):
         queryset = queryset.exclude(cold_storage=True)
     queryset = queryset.filter(date__lt=max_date)[:limit]
 
-    storage = get_storage_class(settings.RTD_BUILD_COMMANDS_STORAGE)()
     for build in queryset:
         data = BuildSerializer(build).data['commands']
         if data:
@@ -172,7 +171,7 @@ def archive_builds_task(days=14, limit=200, include_cold=False, delete=False):
             output.seek(0)
             filename = '{date}/{id}.json'.format(date=str(build.date.date()), id=build.id)
             try:
-                storage.save(name=filename, content=output)
+                build_commands_storage.save(name=filename, content=output)
                 build.cold_storage = True
                 build.save()
                 if delete:

readthedocs/core/utils/general.py

@@ -2,13 +2,12 @@
 
 import os
 
-from django.conf import settings
-from django.core.files.storage import get_storage_class
 from django.shortcuts import get_object_or_404
 
 from readthedocs.core.utils import broadcast
 from readthedocs.projects.tasks import remove_dirs
 from readthedocs.builds.models import Version
+from readthedocs.storage import build_environment_storage
 
 
 def wipe_version_via_slugs(version_slug, project_slug):
@@ -28,5 +27,4 @@ def wipe_version_via_slugs(version_slug, project_slug):
         broadcast(type='build', task=remove_dirs, args=[(del_dir,)])
 
     # Delete the cache environment from storage
-    storage = get_storage_class(settings.RTD_BUILD_ENVIRONMENT_STORAGE)()
-    storage.delete(version.get_storage_environment_cache_path())
+    build_environment_storage.delete(version.get_storage_environment_cache_path())

readthedocs/projects/models.py

@@ -11,7 +11,6 @@
 from django.conf import settings
 from django.conf.urls import include
 from django.contrib.auth.models import User
-from django.core.files.storage import get_storage_class
 from django.core.validators import MaxValueValidator, MinValueValidator
 from django.db import models
 from django.db.models import Prefetch
@@ -46,6 +45,7 @@
 )
 from readthedocs.projects.version_handling import determine_stable_version
 from readthedocs.search.parsers import MkDocsParser, SphinxParser
+from readthedocs.storage import build_media_storage
 from readthedocs.vcs_support.backends import backend_cls
 from readthedocs.vcs_support.utils import Lock, NonBlockingLock
 
@@ -844,12 +844,11 @@ def has_good_build(self):
         return self.builds(manager=INTERNAL).filter(success=True).exists()
 
     def has_media(self, type_, version_slug=LATEST, version_type=None):
-        storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
         storage_path = self.get_storage_path(
             type_=type_, version_slug=version_slug,
             version_type=version_type
         )
-        return storage.exists(storage_path)
+        return build_media_storage.exists(storage_path)
 
     def has_pdf(self, version_slug=LATEST, version_type=None):
         return self.has_media(

readthedocs/projects/tasks.py

@@ -21,7 +21,6 @@
 import requests
 from celery.exceptions import SoftTimeLimitExceeded
 from django.conf import settings
-from django.core.files.storage import get_storage_class
 from django.db.models import Q
 from django.urls import reverse
 from django.utils import timezone
@@ -73,6 +72,7 @@
 from readthedocs.projects.models import APIProject, Feature
 from readthedocs.search.utils import index_new_files, remove_indexed_files
 from readthedocs.sphinx_domains.models import SphinxDomain
+from readthedocs.storage import build_media_storage, build_environment_storage
 from readthedocs.vcs_support import utils as vcs_support_utils
 from readthedocs.worker import app
 
@@ -98,7 +98,6 @@ def pull_cached_environment(self):
         if not self.project.has_feature(feature_id=Feature.CACHED_ENVIRONMENT):
             return
 
-        storage = get_storage_class(settings.RTD_BUILD_ENVIRONMENT_STORAGE)()
         filename = self.version.get_storage_environment_cache_path()
 
         msg = 'Checking for cached environment'
@@ -110,7 +109,7 @@ def pull_cached_environment(self):
                 'msg': msg,
             }
         )
-        if storage.exists(filename):
+        if build_environment_storage.exists(filename):
             msg = 'Pulling down cached environment from storage'
             log.info(
                 LOG_TEMPLATE,
@@ -120,7 +119,7 @@ def pull_cached_environment(self):
                     'msg': msg,
                 }
             )
-            remote_fd = storage.open(filename, mode='rb')
+            remote_fd = build_environment_storage.open(filename, mode='rb')
             with tarfile.open(fileobj=remote_fd) as tar:
                 tar.extractall(self.project.doc_path)
 
@@ -153,7 +152,6 @@ def push_cached_environment(self):
             if os.path.exists(path):
                 tar.add(path, arcname='.cache')
 
-        storage = get_storage_class(settings.RTD_BUILD_ENVIRONMENT_STORAGE)()
         with open(tmp_filename, 'rb') as fd:
             msg = 'Pushing up cached environment to storage'
             log.info(
@@ -164,7 +162,7 @@ def push_cached_environment(self):
                     'msg': msg,
                 }
             )
-            storage.save(
+            build_environment_storage.save(
                 self.version.get_storage_environment_cache_path(),
                 fd,
             )
@@ -1030,7 +1028,6 @@ def store_build_artifacts(
         :param pdf: whether to save PDF output
         :param epub: whether to save ePub output
         """
-        storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
         log.info(
             LOG_TEMPLATE,
             {
@@ -1086,7 +1083,7 @@ def store_build_artifacts(
                 },
             )
             try:
-                storage.sync_directory(from_path, to_path)
+                build_media_storage.sync_directory(from_path, to_path)
             except Exception:
                 # Ideally this should just be an IOError
                 # but some storage backends unfortunately throw other errors
@@ -1115,7 +1112,7 @@ def store_build_artifacts(
                 },
             )
             try:
-                storage.delete_directory(media_path)
+                build_media_storage.delete_directory(media_path)
             except Exception:
                 # Ideally this should just be an IOError
                 # but some storage backends unfortunately throw other errors
@@ -1376,26 +1373,24 @@ def _create_intersphinx_data(version, commit, build):
     if not version.is_sphinx_type:
         return
 
-    storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
-
     html_storage_path = version.project.get_storage_path(
         type_='html', version_slug=version.slug, include_file=False
     )
     json_storage_path = version.project.get_storage_path(
         type_='json', version_slug=version.slug, include_file=False
     )
 
-    object_file = storage.join(html_storage_path, 'objects.inv')
-    if not storage.exists(object_file):
+    object_file = build_media_storage.join(html_storage_path, 'objects.inv')
+    if not build_media_storage.exists(object_file):
         log.debug('No objects.inv, skipping intersphinx indexing.')
         return
 
-    type_file = storage.join(json_storage_path, 'readthedocs-sphinx-domain-names.json')
+    type_file = build_media_storage.join(json_storage_path, 'readthedocs-sphinx-domain-names.json')
     types = {}
     titles = {}
-    if storage.exists(type_file):
+    if build_media_storage.exists(type_file):
         try:
-            data = json.load(storage.open(type_file))
+            data = json.load(build_media_storage.open(type_file))
             types = data['types']
             titles = data['titles']
         except Exception:
@@ -1415,7 +1410,7 @@ def warn(self, msg):
             log.warning('Sphinx MockApp: %s', msg)
 
     # Re-create all objects from the new build of the version
-    object_file_url = storage.url(object_file)
+    object_file_url = build_media_storage.url(object_file)
     if object_file_url.startswith('/'):
         # Filesystem backed storage simply prepends MEDIA_URL to the path to get the URL
         # This can cause an issue if MEDIA_URL is not fully qualified
@@ -1537,15 +1532,13 @@ def _create_imported_files(*, version, commit, build, search_ranking, search_ign
     :returns: paths of changed files
     :rtype: set
     """
-    storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
-
     changed_files = set()
 
     # Re-create all objects from the new build of the version
     storage_path = version.project.get_storage_path(
         type_='html', version_slug=version.slug, include_file=False
     )
-    for root, __, filenames in storage.walk(storage_path):
+    for root, __, filenames in build_media_storage.walk(storage_path):
         for filename in filenames:
             if filename.endswith('.html'):
                 model_class = HTMLFile
@@ -1556,13 +1549,13 @@ def _create_imported_files(*, version, commit, build, search_ranking, search_ign
                 # For projects not behind a CDN, we don't care about non-HTML
                 continue
 
-            full_path = storage.join(root, filename)
+            full_path = build_media_storage.join(root, filename)
 
             # Generate a relative path for storage similar to os.path.relpath
             relpath = full_path.replace(storage_path, '', 1).lstrip('/')
 
             try:
-                md5 = hashlib.md5(storage.open(full_path, 'rb').read()).hexdigest()
+                md5 = hashlib.md5(build_media_storage.open(full_path, 'rb').read()).hexdigest()
             except Exception:
                 log.exception(
                     'Error while generating md5 for %s:%s:%s. Don\'t stop.',
@@ -1808,10 +1801,9 @@ def remove_build_storage_paths(paths):
 
     :param paths: list of paths in build media storage to delete
     """
-    storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
     for storage_path in paths:
         log.info('Removing %s from media storage', storage_path)
-        storage.delete_directory(storage_path)
+        build_media_storage.delete_directory(storage_path)
 
 
 @app.task(queue='web')

readthedocs/projects/views/public.py

@@ -13,7 +13,6 @@
 from django.conf import settings
 from django.contrib import messages
 from django.core.cache import cache
-from django.core.files.storage import get_storage_class
 from django.db.models import prefetch_related_objects
 from django.http import HttpResponse
 from django.shortcuts import get_object_or_404, redirect, render
@@ -37,6 +36,7 @@
 from readthedocs.projects.views.mixins import ProjectRelationListMixin
 from readthedocs.proxito.views.mixins import ServeDocsMixin
 from readthedocs.proxito.views.utils import _get_project_data_from_request
+from readthedocs.storage import build_media_storage
 
 from ..constants import PRIVATE
 from .base import ProjectOnboardMixin
@@ -365,15 +365,14 @@ def get(
             uip=get_client_ip(request),
         )
 
-        storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
         storage_path = version.project.get_storage_path(
             type_=type_,
             version_slug=version_slug,
             version_type=version.type,
         )
 
         # URL without scheme and domain to perform an NGINX internal redirect
-        url = storage.url(storage_path)
+        url = build_media_storage.url(storage_path)
         url = urlparse(url)._replace(scheme='', netloc='').geturl()
 
         return self._serve_docs(

readthedocs/proxito/tests/base.py

@@ -3,17 +3,25 @@
 
 import pytest
 import django_dynamic_fixture as fixture
+from django.conf import settings
 from django.contrib.auth.models import User
+from django.core.files.storage import get_storage_class
 from django.test import TestCase
 
 from readthedocs.projects.constants import PUBLIC
 from readthedocs.projects.models import Project, Domain
+from readthedocs.proxito.views import serve
 
 
 @pytest.mark.proxito
 class BaseDocServing(TestCase):
 
     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)()
+
         self.eric = fixture.get(User, username='eric')
         self.eric.set_password('eric')
         self.eric.save()

readthedocs/proxito/views/mixins.py

@@ -5,7 +5,6 @@
 from slugify import slugify as unicode_slugify
 
 from django.conf import settings
-from django.core.files.storage import get_storage_class
 from django.http import (
     HttpResponse,
     HttpResponseRedirect,
@@ -18,6 +17,7 @@
 from readthedocs.builds.constants import EXTERNAL, INTERNAL
 from readthedocs.core.resolver import resolve
 from readthedocs.redirects.exceptions import InfiniteRedirectException
+from readthedocs.storage import build_media_storage
 
 log = logging.getLogger(__name__)  # noqa
 
@@ -72,8 +72,7 @@ def _serve_docs_python(self, request, final_project, path):
         """
         log.debug('[Django serve] path=%s, project=%s', path, final_project.slug)
 
-        storage = get_storage_class(settings.RTD_BUILD_MEDIA_STORAGE)()
-        root_path = storage.path('')
+        root_path = build_media_storage.path('')
         # Serve from Python
         return serve(request, path, root_path)