Pass build env python limits to config object (#2627)

* Pass build env python limits to config object

* Pass in build env python limitations to config
* Exposes parsing error to user on build output

* Typo on config method

* Alter build image names

* Bump release of readthedocs-build

* Don't test BuildConfig, test our implementation

Tunes up tests, fixes a bug with logic on python version detection

* Fix tests that were depending on config wrapper tests

Author: agjohnson

readthedocs/doc_builder/config.py

@@ -2,6 +2,7 @@
                                       load as load_config)
 
 
+from .constants import BUILD_IMAGES, DOCKER_IMAGE
 from readthedocs.projects.exceptions import ProjectImportError
 
 
@@ -49,25 +50,27 @@ def extra_requirements(self):
 
     @property
     def python_interpreter(self):
-        if 'version' in self._yaml_config.get('python', {}):
-            ver = self._yaml_config['python']['version']
-            if str(ver).startswith('2'):
-                return 'python'
-            else:
-                return 'python3'
-        else:
-            return self._project.python_interpreter
+        ver = self.python_version
+        if ver in [2, 3]:
+            # Get the highest version of the major series version if user only
+            # gave us a version of '2', or '3'
+            ver = max(filter(
+                lambda x: x < ver + 1,
+                self._yaml_config.get_valid_python_versions(),
+            ))
+        return 'python{0}'.format(ver)
 
     @property
     def python_version(self):
+        # There should always be a version in the YAML config. If the config
+        # version is the default response of `2`, then assume we can use the
+        # Python.python_interpreter version to infer this value instead.
+        version = 2
         if 'version' in self._yaml_config.get('python', {}):
-            ver = self._yaml_config['python']['version']
-            return ver
-        else:
-            if self._project.python_interpreter == 'python':
-                return 2
-            else:
-                return 3
+            version = self._yaml_config['python']['version']
+        if version == 2 and self._project.python_interpreter == 'python3':
+            version = 3
+        return version
 
     @property
     def use_system_site_packages(self):
@@ -125,20 +128,36 @@ def load_yaml_config(version):
     """
 
     checkout_path = version.project.checkout_path(version.slug)
+    env_config = {}
+
+    # Get build image to set up the python version validation. Pass in the
+    # build image python limitations to the loaded config so that the versions
+    # can be rejected at validation
+    build_image = BUILD_IMAGES.get(
+        version.project.container_image,
+        BUILD_IMAGES.get(DOCKER_IMAGE, None),
+    )
+    if build_image:
+        env_config = {
+            'python': build_image['python'],
+        }
+
     try:
+        sphinx_env_config = env_config.copy()
+        sphinx_env_config.update({
+            'output_base': '',
+            'type': 'sphinx',
+            'name': version.slug,
+        })
         config = load_config(
             path=checkout_path,
-            env_config={
-                'output_base': '',
-                'type': 'sphinx',
-                'name': version.slug,
-            },
+            env_config=sphinx_env_config,
         )[0]
     except InvalidConfig:  # This is a subclass of ConfigError, so has to come first
         raise
     except ConfigError:
         config = BuildConfig(
-            env_config={},
+            env_config=env_config,
             raw_config={},
             source_file='empty',
             source_position=0,

readthedocs/doc_builder/constants.py

@@ -15,13 +15,27 @@
 
 PDF_RE = re.compile('Output written on (.*?)')
 
+# Docker
 DOCKER_SOCKET = getattr(settings, 'DOCKER_SOCKET', 'unix:///var/run/docker.sock')
 DOCKER_VERSION = getattr(settings, 'DOCKER_VERSION', 'auto')
-DOCKER_IMAGE = getattr(settings, 'DOCKER_IMAGE', 'rtfd-build')
+DOCKER_IMAGE = getattr(settings, 'DOCKER_IMAGE', 'readthedocs/build:2.0')
 DOCKER_LIMITS = {'memory': '200m', 'time': 600}
 DOCKER_LIMITS.update(getattr(settings, 'DOCKER_LIMITS', {}))
 
 DOCKER_TIMEOUT_EXIT_CODE = 42
 DOCKER_OOM_EXIT_CODE = 137
 
 DOCKER_HOSTNAME_MAX_LEN = 64
+
+# Build images
+BUILD_IMAGES = {
+    'readthedocs/build:1.0': {
+        'python': {'supported_versions': [2, 2.7, 3, 3.3]},
+    },
+    'readthedocs/build:2.0': {
+        'python': {'supported_versions': [2, 2.7, 3, 3.5]},
+    },
+    'readthedocs/build:latest': {
+        'python': {'supported_versions': [2, 2.7, 3, 3.3, 3.4, 3.5, 3.6]},
+    },
+}

readthedocs/projects/tasks.py

@@ -18,6 +18,7 @@
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext_lazy as _
+from readthedocs_build.config import ConfigError
 
 from readthedocs.builds.constants import (LATEST,
                                           BUILD_STATE_CLONING,
@@ -132,7 +133,12 @@ def run(self, pk, version_pk=None, build_pk=None, record=True, docker=False,
                     status_code=423
                 )
 
-            self.config = load_yaml_config(version=self.version)
+            try:
+                self.config = load_yaml_config(version=self.version)
+            except ConfigError as e:
+                raise BuildEnvironmentError(
+                    'Problem parsing YAML configuration. {0}'.format(str(e))
+                )
 
         if self.setup_env.failure or self.config is None:
             self._log('Failing build because of setup failure: %s' % self.setup_env.failure)

readthedocs/rtd_tests/tests/test_builds.py

@@ -10,7 +10,7 @@
 from readthedocs.doc_builder.python_environments import Virtualenv
 from readthedocs.doc_builder.loader import get_builder_class
 from readthedocs.projects.tasks import UpdateDocsTask
-from readthedocs.rtd_tests.tests.test_config_wrapper import get_build_config
+from readthedocs.rtd_tests.tests.test_config_wrapper import create_load
 
 from ..mocks.environment import EnvironmentMockGroup
 
@@ -40,8 +40,7 @@ def test_build(self):
 
         build_env = LocalEnvironment(project=project, version=version, build={})
         python_env = Virtualenv(version=version, build_env=build_env)
-        yaml_config = get_build_config({})
-        config = ConfigWrapper(version=version, yaml_config=yaml_config)
+        config = ConfigWrapper(version=version, yaml_config=create_load()()[0])
         task = UpdateDocsTask(build_env=build_env, project=project, python_env=python_env,
                               version=version, search=False, localmedia=False, config=config)
         task.build_docs()
@@ -65,8 +64,7 @@ def test_build_respects_pdf_flag(self):
 
         build_env = LocalEnvironment(project=project, version=version, build={})
         python_env = Virtualenv(version=version, build_env=build_env)
-        yaml_config = get_build_config({})
-        config = ConfigWrapper(version=version, yaml_config=yaml_config)
+        config = ConfigWrapper(version=version, yaml_config=create_load()()[0])
         task = UpdateDocsTask(build_env=build_env, project=project, python_env=python_env,
                               version=version, search=False, localmedia=False, config=config)
 
@@ -91,8 +89,7 @@ def test_build_respects_epub_flag(self):
 
         build_env = LocalEnvironment(project=project, version=version, build={})
         python_env = Virtualenv(version=version, build_env=build_env)
-        yaml_config = get_build_config({})
-        config = ConfigWrapper(version=version, yaml_config=yaml_config)
+        config = ConfigWrapper(version=version, yaml_config=create_load()()[0])
         task = UpdateDocsTask(build_env=build_env, project=project, python_env=python_env,
                               version=version, search=False, localmedia=False, config=config)
         task.build_docs()
@@ -116,8 +113,9 @@ def test_build_respects_yaml(self):
 
         build_env = LocalEnvironment(project=project, version=version, build={})
         python_env = Virtualenv(version=version, build_env=build_env)
-        yaml_config = get_build_config({'formats': ['epub']})
-        config = ConfigWrapper(version=version, yaml_config=yaml_config)
+        config = ConfigWrapper(version=version, yaml_config=create_load({
+            'formats': ['epub']
+        })()[0])
         task = UpdateDocsTask(build_env=build_env, project=project, python_env=python_env,
                               version=version, search=False, localmedia=False, config=config)
         task.build_docs()
@@ -171,8 +169,7 @@ def test_build_pdf_latex_failures(self):
 
         build_env = LocalEnvironment(project=project, version=version, build={})
         python_env = Virtualenv(version=version, build_env=build_env)
-        yaml_config = get_build_config({})
-        config = ConfigWrapper(version=version, yaml_config=yaml_config)
+        config = ConfigWrapper(version=version, yaml_config=create_load()()[0])
         task = UpdateDocsTask(build_env=build_env, project=project, python_env=python_env,
                               version=version, search=False, localmedia=False, config=config)
 
@@ -213,8 +210,7 @@ def test_build_pdf_latex_not_failure(self):
 
         build_env = LocalEnvironment(project=project, version=version, build={})
         python_env = Virtualenv(version=version, build_env=build_env)
-        yaml_config = get_build_config({})
-        config = ConfigWrapper(version=version, yaml_config=yaml_config)
+        config = ConfigWrapper(version=version, yaml_config=create_load()()[0])
         task = UpdateDocsTask(build_env=build_env, project=project, python_env=python_env,
                               version=version, search=False, localmedia=False, config=config)