Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build: implement build.jobs config file key #9016

Merged
merged 17 commits into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion readthedocs/config/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from .find import find_one
from .models import (
Build,
BuildJobs,
BuildTool,
BuildWithTools,
Conda,
Expand Down Expand Up @@ -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).
Expand All @@ -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',
Expand All @@ -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}'):
Expand Down Expand Up @@ -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)

Expand Down
23 changes: 22 additions & 1 deletion readthedocs/config/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', [])
Expand All @@ -49,6 +49,27 @@ class BuildTool(Base):
__slots__ = ('version', 'full_version')


class BuildJobs(Base):

__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):
for step in self.__slots__:
kwargs.setdefault(step, [])
super().__init__(**kwargs)


class Python(Base):

__slots__ = ('version', 'install', 'use_system_site_packages')
Expand Down
96 changes: 95 additions & 1 deletion readthedocs/config/tests/test_config.py
Original file line number Diff line number Diff line change
@@ -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 (
Expand Down Expand Up @@ -33,6 +33,7 @@
)
from readthedocs.config.models import (
Build,
BuildJobs,
BuildWithTools,
Conda,
PythonInstall,
Expand Down Expand Up @@ -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',
[
Expand Down Expand Up @@ -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,
Expand Down
Loading