Merge pull request #9016 from readthedocs/humitos/build-jobs-config-object

Author: humitos

readthedocs/config/config.py

@@ -15,6 +15,7 @@
 from .find import find_one
 from .models import (
     Build,
+    BuildJobs,
     BuildTool,
     BuildWithTools,
     Conda,
@@ -751,6 +752,10 @@ def validate_conda(self):
             conda['environment'] = validate_path(environment, self.base_path)
         return conda
 
+    # NOTE: I think we should rename `BuildWithTools` to `BuildWithOs` since
+    # `os` is the main and mandatory key that makes the diference
+    #
+    # NOTE: `build.jobs` can't be used without using `build.os`
     def validate_build_config_with_tools(self):
         """
         Validates the build object (new format).
@@ -769,6 +774,22 @@ def validate_build_config_with_tools(self):
             for tool in tools.keys():
                 validate_choice(tool, self.settings['tools'].keys())
 
+        jobs = {}
+        with self.catch_validation_error("build.jobs"):
+            # FIXME: should we use `default={}` or kept the `None` here and
+            # shortcircuit the rest of the logic?
+            jobs = self.pop_config("build.jobs", default={})
+            validate_dict(jobs)
+            # NOTE: besides validating that each key is one of the expected
+            # ones, we could validate the value of each of them is a list of
+            # commands. However, I don't think we should validate the "command"
+            # looks like a real command.
+            for job in jobs.keys():
+                validate_choice(
+                    job,
+                    BuildJobs.__slots__,
+                )
+
         if not tools:
             self.error(
                 key='build.tools',
@@ -780,6 +801,13 @@ def validate_build_config_with_tools(self):
                 code=CONFIG_REQUIRED,
             )
 
+        build["jobs"] = {}
+        for job, commands in jobs.items():
+            with self.catch_validation_error(f"build.jobs.{job}"):
+                build["jobs"][job] = [
+                    validate_string(command) for command in validate_list(commands)
+                ]
+
         build['tools'] = {}
         for tool, version in tools.items():
             with self.catch_validation_error(f'build.tools.{tool}'):
@@ -1263,7 +1291,8 @@ def build(self):
             return BuildWithTools(
                 os=build['os'],
                 tools=tools,
-                apt_packages=build['apt_packages'],
+                jobs=BuildJobs(**build["jobs"]),
+                apt_packages=build["apt_packages"],
             )
         return Build(**build)
 

readthedocs/config/models.py

@@ -37,7 +37,7 @@ def __init__(self, **kwargs):
 
 class BuildWithTools(Base):
 
-    __slots__ = ('os', 'tools', 'apt_packages')
+    __slots__ = ("os", "tools", "jobs", "apt_packages")
 
     def __init__(self, **kwargs):
         kwargs.setdefault('apt_packages', [])
@@ -49,6 +49,35 @@ class BuildTool(Base):
     __slots__ = ('version', 'full_version')
 
 
+class BuildJobs(Base):
+
+    """Object used for `build.jobs` key."""
+
+    __slots__ = (
+        "pre_checkout",
+        "post_checkout",
+        "pre_system_dependencies",
+        "post_system_dependencies",
+        "pre_create_environment",
+        "post_create_environment",
+        "pre_install",
+        "post_install",
+        "pre_build",
+        "post_build",
+    )
+
+    def __init__(self, **kwargs):
+        """
+        Create an empty list as a default for all possible builds.jobs configs.
+
+        This is necessary because it makes the code cleaner when we add items to these lists,
+        without having to check for a dict to be created first.
+        """
+        for step in self.__slots__:
+            kwargs.setdefault(step, [])
+        super().__init__(**kwargs)
+
+
 class Python(Base):
 
     __slots__ = ('version', 'install', 'use_system_site_packages')

readthedocs/config/tests/test_config.py

@@ -1,11 +1,11 @@
 import os
-from django.conf import settings
 import re
 import textwrap
 from collections import OrderedDict
 from unittest.mock import DEFAULT, patch
 
 import pytest
+from django.conf import settings
 from pytest import raises
 
 from readthedocs.config import (
@@ -33,6 +33,7 @@
 )
 from readthedocs.config.models import (
     Build,
+    BuildJobs,
     BuildWithTools,
     Conda,
     PythonInstall,
@@ -1012,6 +1013,87 @@ def test_new_build_config_conflict_with_build_python_version(self):
             build.validate()
         assert excinfo.value.key == 'python.version'
 
+    @pytest.mark.parametrize("value", ["", None, "pre_invalid"])
+    def test_jobs_build_config_invalid_jobs(self, value):
+        build = self.get_build_config(
+            {
+                "build": {
+                    "os": "ubuntu-20.04",
+                    "tools": {"python": "3.8"},
+                    "jobs": {value: ["echo 1234", "git fetch --unshallow"]},
+                },
+            },
+        )
+        with raises(InvalidConfig) as excinfo:
+            build.validate()
+        assert excinfo.value.key == "build.jobs"
+
+    @pytest.mark.parametrize("value", ["", None, "echo 123", 42])
+    def test_jobs_build_config_invalid_job_commands(self, value):
+        build = self.get_build_config(
+            {
+                "build": {
+                    "os": "ubuntu-20.04",
+                    "tools": {"python": "3.8"},
+                    "jobs": {
+                        "pre_install": value,
+                    },
+                },
+            },
+        )
+        with raises(InvalidConfig) as excinfo:
+            build.validate()
+        assert excinfo.value.key == "build.jobs.pre_install"
+
+    def test_jobs_build_config(self):
+        build = self.get_build_config(
+            {
+                "build": {
+                    "os": "ubuntu-20.04",
+                    "tools": {"python": "3.8"},
+                    "jobs": {
+                        "pre_checkout": ["echo pre_checkout"],
+                        "post_checkout": ["echo post_checkout"],
+                        "pre_system_dependencies": ["echo pre_system_dependencies"],
+                        "post_system_dependencies": ["echo post_system_dependencies"],
+                        "pre_create_environment": ["echo pre_create_environment"],
+                        "post_create_environment": ["echo post_create_environment"],
+                        "pre_install": ["echo pre_install", "echo `date`"],
+                        "post_install": ["echo post_install"],
+                        "pre_build": [
+                            "echo pre_build",
+                            'sed -i -e "s|{VERSION}|${READTHEDOCS_VERSION_NAME}|g"',
+                        ],
+                        "post_build": ["echo post_build"],
+                    },
+                },
+            },
+        )
+        build.validate()
+        assert isinstance(build.build, BuildWithTools)
+        assert isinstance(build.build.jobs, BuildJobs)
+        assert build.build.jobs.pre_checkout == ["echo pre_checkout"]
+        assert build.build.jobs.post_checkout == ["echo post_checkout"]
+        assert build.build.jobs.pre_system_dependencies == [
+            "echo pre_system_dependencies"
+        ]
+        assert build.build.jobs.post_system_dependencies == [
+            "echo post_system_dependencies"
+        ]
+        assert build.build.jobs.pre_create_environment == [
+            "echo pre_create_environment"
+        ]
+        assert build.build.jobs.post_create_environment == [
+            "echo post_create_environment"
+        ]
+        assert build.build.jobs.pre_install == ["echo pre_install", "echo `date`"]
+        assert build.build.jobs.post_install == ["echo post_install"]
+        assert build.build.jobs.pre_build == [
+            "echo pre_build",
+            'sed -i -e "s|{VERSION}|${READTHEDOCS_VERSION_NAME}|g"',
+        ]
+        assert build.build.jobs.post_build == ["echo post_build"]
+
     @pytest.mark.parametrize(
         'value',
         [
@@ -2297,6 +2379,18 @@ def test_as_dict_new_build_config(self, tmpdir):
                         'full_version': settings.RTD_DOCKER_BUILD_SETTINGS['tools']['nodejs']['16'],
                     },
                 },
+                "jobs": {
+                    "pre_checkout": [],
+                    "post_checkout": [],
+                    "pre_system_dependencies": [],
+                    "post_system_dependencies": [],
+                    "pre_create_environment": [],
+                    "post_create_environment": [],
+                    "pre_install": [],
+                    "post_install": [],
+                    "pre_build": [],
+                    "post_build": [],
+                },
                 'apt_packages': [],
             },
             'conda': None,