Build: pass api_client down to environment/builders/etc

For #10289
we won't have the credentials hardcoded in our settings anymore,
but instead we will be provided with a dynamic token for the
build to use, for this reason we now have to pass the api client
object around.

Author: stsewd

readthedocs/api/v2/client.py

@@ -1,8 +1,7 @@
 """Simple client to access our API with Slumber credentials."""
 
-import structlog
-
 import requests
+import structlog
 from django.conf import settings
 from rest_framework.renderers import JSONRenderer
 from slumber import API, serialize
@@ -71,6 +70,3 @@ def setup_api():
     else:
         log.warning('SLUMBER_USERNAME/PASSWORD settings are not set')
     return API(**api_config)
-
-
-api = setup_api()

readthedocs/doc_builder/backends/sphinx.py

@@ -16,7 +16,6 @@
 from django.urls import reverse
 from requests.exceptions import ConnectionError
 
-from readthedocs.api.v2.client import api
 from readthedocs.builds import utils as version_utils
 from readthedocs.core.utils.filesystem import safe_open
 from readthedocs.doc_builder.exceptions import PDFNotFound
@@ -167,33 +166,23 @@ def get_config_params(self):
         versions = []
         downloads = []
         subproject_urls = []
-        # Avoid hitting database and API if using Docker build environment
-        if settings.DONT_HIT_API:
-            if self.project.has_feature(Feature.ALL_VERSIONS_IN_HTML_CONTEXT):
-                versions = self.project.active_versions()
-            else:
-                versions = self.project.active_versions().filter(
-                    privacy_level=PUBLIC,
-                )
-            downloads = self.version.get_downloads(pretty=True)
-            subproject_urls = self.project.get_subproject_urls()
-        else:
-            try:
-                versions = self.project.api_versions()
-                if not self.project.has_feature(Feature.ALL_VERSIONS_IN_HTML_CONTEXT):
-                    versions = [
-                        v
-                        for v in versions
-                        if v.privacy_level == PUBLIC
-                    ]
-                downloads = api.version(self.version.pk).get()['downloads']
-                subproject_urls = self.project.get_subproject_urls()
-            except ConnectionError:
-                log.exception(
-                    'Timeout while fetching versions/downloads/subproject_urls for Sphinx context.',
-                    project_slug=self.project.slug,
-                    version_slug=self.version.slug,
-                )
+        try:
+            versions = self.project.api_versions(self.api_client)
+            if not self.project.has_feature(Feature.ALL_VERSIONS_IN_HTML_CONTEXT):
+                versions = [v for v in versions if v.privacy_level == PUBLIC]
+            downloads = self.api_client.version(self.version.pk).get()["downloads"]
+            subproject_urls = [
+                (project["slug"], project["canonical_url"])
+                for project in self.api_client.project(self.project.pk)
+                .subprojects()
+                .get()["subprojects"]
+            ]
+        except ConnectionError:
+            log.exception(
+                "Timeout while fetching versions/downloads/subproject_urls for Sphinx context.",
+                project_slug=self.project.slug,
+                version_slug=self.version.slug,
+            )
 
         build_id = self.build_env.build.get('id')
         build_url = None
@@ -304,7 +293,7 @@ def append_conf(self):
 
         # Append config to project conf file
         tmpl = template_loader.get_template('doc_builder/conf.py.tmpl')
-        rendered = tmpl.render(self.get_config_params())
+        rendered = tmpl.render(self.get_config_params(self.api_client))
 
         with outfile:
             outfile.write('\n')

readthedocs/doc_builder/base.py

@@ -37,6 +37,7 @@ def __init__(self, build_env, python_env):
         self.project = build_env.project
         self.config = python_env.config if python_env else None
         self.project_path = self.project.checkout_path(self.version.slug)
+        self.api_client = self.build_env.api_client
 
     def get_final_doctype(self):
         """Some builders may have a different doctype at build time."""
@@ -52,11 +53,8 @@ def build(self):
     def _post_build(self):
         """Execute extra steps (e.g. create ZIP, rename PDF, etc) after building if required."""
 
-    def docs_dir(self, docs_dir=None, **__):
+    def docs_dir(self):
         """Handle creating a custom docs_dir if it doesn't exist."""
-        if docs_dir:
-            return docs_dir
-
         for doc_dir_name in ['docs', 'doc', 'Doc', 'book']:
             possible_path = os.path.join(self.project_path, doc_dir_name)
             if os.path.exists(possible_path):

readthedocs/doc_builder/director.py

@@ -121,6 +121,7 @@ def create_vcs_environment(self):
             # Force the ``container_image`` to use one that has the latest
             # ca-certificate package which is compatible with Lets Encrypt
             container_image=settings.RTD_DOCKER_BUILD_SETTINGS["os"]["ubuntu-20.04"],
+            api_client=self.data.api_client,
         )
 
     def create_build_environment(self):
@@ -132,6 +133,7 @@ def create_build_environment(self):
             build=self.data.build,
             environment=self.get_build_env_vars(),
             use_gvisor=use_gvisor,
+            api_client=self.data.api_client,
         )
 
     def setup_environment(self):
@@ -588,7 +590,7 @@ def build_docs_class(self, builder_class):
         )
 
         if builder_class == self.data.config.doctype:
-            builder.append_conf()
+            builder.append_conf(self.data.api_client)
             self.data.version.documentation_type = builder.get_final_doctype()
 
         success = builder.build()

readthedocs/doc_builder/environments.py

@@ -17,7 +17,6 @@
 from requests.exceptions import ConnectionError, ReadTimeout
 from requests_toolbelt.multipart.encoder import MultipartEncoder
 
-from readthedocs.api.v2.client import api as api_v2
 from readthedocs.builds.models import BuildCommandResultMixin
 from readthedocs.core.utils import slugify
 from readthedocs.projects.models import Feature
@@ -227,7 +226,7 @@ def get_command(self):
             return ' '.join(self.command)
         return self.command
 
-    def save(self):
+    def save(self, api_client):
         """Save this command and result via the API."""
         # Force record this command as success to avoid Build reporting errors
         # on commands that are just for checking purposes and do not interferes
@@ -251,7 +250,7 @@ def save(self):
             encoder = MultipartEncoder(
                 {key: str(value) for key, value in data.items()}
             )
-            resource = api_v2.command
+            resource = api_client.command
             resp = resource._store["session"].post(
                 resource._store["base_url"] + "/",
                 data=encoder,
@@ -261,7 +260,7 @@ def save(self):
             )
             log.debug('Post response via multipart form.', response=resp)
         else:
-            resp = api_v2.command.post(data)
+            resp = api_client.command.post(data)
             log.debug('Post response via JSON encoded data.', response=resp)
 
 
@@ -416,9 +415,12 @@ class BaseBuildEnvironment:
     :param build: Build instance
     :param environment: shell environment variables
     :param record: whether or not record a build commands in the databse via
-    the API. The only case where we want this to be `False` is when
-    instantiating this class from `sync_repository_task` because it's a
-    background task that does not expose commands to the user.
+     the API. The only case where we want this to be `False` is when
+     instantiating this class from `sync_repository_task` because it's a
+     background task that does not expose commands to the user.
+    :param api_client: API v2 client instance (readthedocs.v2.client).
+     This is used to record commands in the database, if `record=True`
+     this argument is required.
     """
 
     def __init__(
@@ -429,6 +431,7 @@ def __init__(
         config=None,
         environment=None,
         record=True,
+        api_client=None,
         **kwargs,
     ):
         self.project = project
@@ -438,6 +441,10 @@ def __init__(
         self.build = build
         self.config = config
         self.record = record
+        self.api_client = api_client
+
+        if self.record and not self.api_client:
+            raise ValueError("api_client is required when record=True")
 
     # TODO: remove these methods, we are not using LocalEnvironment anymore. We
     # need to find a way for tests to not require this anymore
@@ -449,7 +456,7 @@ def __exit__(self, exc_type, exc_value, tb):
 
     def record_command(self, command):
         if self.record:
-            command.save()
+            command.save(self.api_client)
 
     def run(self, *cmd, **kwargs):
         """Shortcut to run command from environment."""

readthedocs/projects/models.py

@@ -25,7 +25,6 @@
 from django_extensions.db.models import TimeStampedModel
 from taggit.managers import TaggableManager
 
-from readthedocs.api.v2.client import api
 from readthedocs.builds.constants import EXTERNAL, INTERNAL, LATEST, STABLE
 from readthedocs.constants import pattern_opts
 from readthedocs.core.history import ExtraHistoricalRecords
@@ -562,23 +561,6 @@ def get_builds_url(self):
             },
         )
 
-    def get_canonical_url(self):
-        if settings.DONT_HIT_DB:
-            return api.project(self.pk).canonical_url().get()['url']
-        return self.get_docs_url()
-
-    def get_subproject_urls(self):
-        """
-        List subproject URLs.
-
-        This is used in search result linking
-        """
-        if settings.DONT_HIT_DB:
-            return [(proj['slug'], proj['canonical_url']) for proj in
-                    (api.project(self.pk).subprojects().get()['subprojects'])]
-        return [(proj.child.slug, proj.child.get_docs_url())
-                for proj in self.subprojects.all()]
-
     def get_storage_paths(self):
         """
         Get the paths of all artifacts used by the project.
@@ -1047,13 +1029,17 @@ def get_latest_build(self, finished=True):
             kwargs['state'] = 'finished'
         return self.builds(manager=INTERNAL).filter(**kwargs).first()
 
-    def api_versions(self):
+    def api_versions(self, api_client):
+        """
+        Get active versions from the API.
+
+        :param api_client: API v2 client instance (readthedocs.v2.client).
+        """
         from readthedocs.builds.models import APIVersion
-        ret = []
-        for version_data in api.project(self.pk).active_versions.get()['versions']:
-            version = APIVersion(**version_data)
-            ret.append(version)
-        return sort_version_aware(ret)
+
+        versions_data = api_client.project(self.pk).active_versions.get()["versions"]
+        versions = [APIVersion(**version_data) for version_data in versions_data]
+        return sort_version_aware(versions)
 
     def active_versions(self):
         from readthedocs.builds.models import Version

readthedocs/projects/tasks/builds.py

@@ -13,9 +13,10 @@
 from celery import Task
 from django.conf import settings
 from django.utils import timezone
+from slumber import API
 from slumber.exceptions import HttpClientError
 
-from readthedocs.api.v2.client import api as api_v2
+from readthedocs.api.v2.client import setup_api
 from readthedocs.builds import tasks as build_tasks
 from readthedocs.builds.constants import (
     ARTIFACT_TYPES,
@@ -102,6 +103,9 @@ class TaskData:
     build_pk: int = None
     build_commit: str = None
 
+    # Slumber client to interact with the API v2.
+    api_client: API = None
+
     start_time: timezone.datetime = None
     # pylint: disable=unsubscriptable-object
     environment_class: type[DockerBuildEnvironment] | type[LocalBuildEnvironment] = None
@@ -153,6 +157,8 @@ def before_start(self, task_id, args, kwargs):
         # argument
         self.data.version_pk = args[0]
 
+        self.data.api_client = setup_api()
+
         # load all data from the API required for the build
         self.data.version = self.get_version(self.data.version_pk)
         self.data.project = self.data.version.project
@@ -339,8 +345,10 @@ def sigint_received(*args, **kwargs):
 
     def _check_concurrency_limit(self):
         try:
-            response = api_v2.build.concurrent.get(project__slug=self.data.project.slug)
-            concurrency_limit_reached = response.get('limit_reached', False)
+            response = self.data.api_client.build.concurrent.get(
+                project__slug=self.data.project.slug
+            )
+            concurrency_limit_reached = response.get("limit_reached", False)
             max_concurrent_builds = response.get(
                 'max_concurrent',
                 settings.RTD_MAX_CONCURRENT_BUILDS,
@@ -390,6 +398,8 @@ def before_start(self, task_id, args, kwargs):
             # anymore and we are not using it
             self.data.environment_class = LocalBuildEnvironment
 
+        self.data.api_client = setup_api()
+
         self.data.build = self.get_build(self.data.build_pk)
         self.data.version = self.get_version(self.data.version_pk)
         self.data.project = self.data.version.project
@@ -427,7 +437,7 @@ def _reset_build(self):
         # Reset build only if it has some commands already.
         if self.data.build.get("commands"):
             log.info("Resetting build.")
-            api_v2.build(self.data.build["id"]).reset.post()
+            self.data.api_client.build(self.data.build["id"]).reset.post()
 
     def on_failure(self, exc, task_id, args, kwargs, einfo):
         """
@@ -598,7 +608,7 @@ def on_success(self, retval, task_id, args, kwargs):
         # TODO: remove this condition and *always* update the DB Version instance
         if "html" in valid_artifacts:
             try:
-                api_v2.version(self.data.version.pk).patch(
+                self.data.api_client.version(self.data.version.pk).patch(
                     {
                         "built": True,
                         "documentation_type": self.data.version.documentation_type,
@@ -709,7 +719,7 @@ def update_build(self, state=None):
         #         self.data.build[key] = val.decode('utf-8', 'ignore')
 
         try:
-            api_v2.build(self.data.build['id']).patch(self.data.build)
+            self.data.api_client.build(self.data.build["id"]).patch(self.data.build)
         except Exception:
             # NOTE: we are updating the "Build" object on each `state`.
             # Only if the last update fails, there may be some inconsistency
@@ -802,22 +812,15 @@ def save_build_data(self):
         except Exception:
             log.exception("Error while saving build data")
 
-    @staticmethod
-    def get_project(project_pk):
-        """Get project from API."""
-        project_data = api_v2.project(project_pk).get()
-        return APIProject(**project_data)
-
-    @staticmethod
-    def get_build(build_pk):
+    def get_build(self, build_pk):
         """
         Retrieve build object from API.
 
         :param build_pk: Build primary key
         """
         build = {}
         if build_pk:
-            build = api_v2.build(build_pk).get()
+            build = self.data.api_client.build(build_pk).get()
         private_keys = [
             'project',
             'version',
@@ -834,7 +837,7 @@ def get_build(build_pk):
     # build has finished to reduce API calls.
     def set_valid_clone(self):
         """Mark on the project that it has been cloned properly."""
-        api_v2.project(self.data.project.pk).patch(
+        self.data.api_client.project(self.data.project.pk).patch(
             {'has_valid_clone': True}
         )
         self.data.project.has_valid_clone = True