Merge pull request #4749 from stsewd/save-config-on-build

Save config on build model

Author: ericholscher

readthedocs/builds/migrations/0006_add_config_field.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.16 on 2018-11-02 13:24
+from __future__ import unicode_literals
+
+from django.db import migrations
+import jsonfield.fields
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('builds', '0005_remove-version-alias'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='build',
+            name='_config',
+            field=jsonfield.fields.JSONField(default=dict, verbose_name='Configuration used in the build'),
+        ),
+    ]

readthedocs/builds/models.py

@@ -17,10 +17,12 @@
 from django.conf import settings
 from django.core.urlresolvers import reverse
 from django.db import models
+from django.utils import timezone
 from django.utils.encoding import python_2_unicode_compatible
 from django.utils.translation import ugettext
 from django.utils.translation import ugettext_lazy as _
 from guardian.shortcuts import assign
+from jsonfield import JSONField
 from taggit.managers import TaggableManager
 
 from readthedocs.core.utils import broadcast
@@ -128,6 +130,21 @@ def __str__(self):
                 pk=self.pk,
             ))
 
+    @property
+    def config(self):
+        """
+        Proxy to the configuration of the build.
+
+        :returns: The configuration used in the last successful build.
+        :rtype: dict
+        """
+        last_build = (
+            self.builds.filter(state='finished', success=True)
+            .order_by('-date')
+            .first()
+        )
+        return last_build.config
+
     @property
     def commit_name(self):
         """
@@ -450,6 +467,7 @@ class Build(models.Model):
     exit_code = models.IntegerField(_('Exit code'), null=True, blank=True)
     commit = models.CharField(
         _('Commit'), max_length=255, null=True, blank=True)
+    _config = JSONField(_('Configuration used in the build'), default=dict)
 
     length = models.IntegerField(_('Build Length'), null=True, blank=True)
 
@@ -463,11 +481,81 @@ class Build(models.Model):
 
     objects = BuildQuerySet.as_manager()
 
+    CONFIG_KEY = '__config'
+
     class Meta(object):
         ordering = ['-date']
         get_latest_by = 'date'
         index_together = [['version', 'state', 'type']]
 
+    def __init__(self, *args, **kwargs):
+        super(Build, self).__init__(*args, **kwargs)
+        self._config_changed = False
+
+    @property
+    def previous(self):
+        """
+        Returns the previous build to the current one.
+
+        Matching the project and version.
+        """
+        date = self.date or timezone.now()
+        if self.project is not None and self.version is not None:
+            return (
+                Build.objects
+                .filter(
+                    project=self.project,
+                    version=self.version,
+                    date__lt=date,
+                )
+                .order_by('-date')
+                .first()
+            )
+        return None
+
+    @property
+    def config(self):
+        """
+        Get the config used for this build.
+
+        Since we are saving the config into the JSON field only when it differs
+        from the previous one, this helper returns the correct JSON used in
+        this Build object (it could be stored in this object or one of the
+        previous ones).
+        """
+        if self.CONFIG_KEY in self._config:
+            return Build.objects.get(pk=self._config[self.CONFIG_KEY])._config
+        return self._config
+
+    @config.setter
+    def config(self, value):
+        """
+        Set `_config` to value.
+
+        `_config` should never be set directly from outside the class.
+        """
+        self._config = value
+        self._config_changed = True
+
+    def save(self, *args, **kwargs):  # noqa
+        """
+        Save object.
+
+        To save space on the db we only save the config if it's different
+        from the previous one.
+
+        If the config is the same, we save the pk of the object
+        that has the **real** config under the `CONFIG_KEY` key.
+        """
+        if self.pk is None or self._config_changed:
+            previous = self.previous
+            if (previous is not None and
+                    self._config and self._config == previous.config):
+                previous_pk = previous._config.get(self.CONFIG_KEY, previous.pk)
+                self._config = {self.CONFIG_KEY: previous_pk}
+        super(Build, self).save(*args, **kwargs)
+        self._config_changed = False
+
     def __str__(self):
         return ugettext(
             'Build {project} for {usernames} ({pk})'.format(

readthedocs/config/config.py

@@ -142,6 +142,11 @@ class BuildConfigBase(object):
                         from another source (like the web admin).
     """
 
+    PUBLIC_ATTRIBUTES = [
+        'version', 'formats', 'python',
+        'conda', 'build', 'doctype',
+        'sphinx', 'mkdocs', 'submodules',
+    ]
     version = None
 
     def __init__(self, env_config, raw_config, source_file, source_position):
@@ -248,6 +253,16 @@ def python_full_version(self):
             )
         return ver
 
+    def as_dict(self):
+        config = {}
+        for name in self.PUBLIC_ATTRIBUTES:
+            attr = getattr(self, name)
+            if hasattr(attr, '_asdict'):
+                config[name] = attr._asdict()
+            else:
+                config[name] = attr
+        return config
+
     def __getattr__(self, name):
         """Raise an error for unknown attributes."""
         raise ConfigOptionNotSupportedError(name)

readthedocs/config/tests/test_config.py

@@ -840,6 +840,60 @@ def test_config_filenames_regex(correct_config_filename):
     assert re.match(CONFIG_FILENAME_REGEX, correct_config_filename)
 
 
+def test_as_dict(tmpdir):
+    apply_fs(tmpdir, {'requirements.txt': ''})
+    build = get_build_config(
+        {
+            'version': 1,
+            'formats': ['pdf'],
+            'python': {
+                'version': 3.5,
+            },
+            'requirements_file': 'requirements.txt',
+        },
+        get_env_config({
+            'defaults': {
+                'doctype': 'sphinx',
+                'sphinx_configuration': None,
+            },
+        }),
+        source_file=str(tmpdir.join('readthedocs.yml')),
+    )
+    build.validate()
+    expected_dict = {
+        'version': '1',
+        'formats': ['pdf'],
+        'python': {
+            'version': 3.5,
+            'requirements': 'requirements.txt',
+            'install_with_pip': False,
+            'install_with_setup': False,
+            'extra_requirements': [],
+            'use_system_site_packages': False,
+        },
+        'build': {
+            'image': 'readthedocs/build:2.0',
+        },
+        'conda': None,
+        'sphinx': {
+            'builder': 'sphinx',
+            'configuration': None,
+            'fail_on_warning': False,
+        },
+        'mkdocs': {
+            'configuration': None,
+            'fail_on_warning': False,
+        },
+        'doctype': 'sphinx',
+        'submodules': {
+            'include': ALL,
+            'exclude': [],
+            'recursive': True,
+        },
+    }
+    assert build.as_dict() == expected_dict
+
+
 class TestBuildConfigV2(object):
 
     def get_build_config(self, config, env_config=None,
@@ -1811,3 +1865,47 @@ def test_pop_config_raise_exception(self):
             build.pop_config('one.four', raise_ex=True)
         assert excinfo.value.value == 'four'
         assert excinfo.value.code == VALUE_NOT_FOUND
+
+    def test_as_dict(self, tmpdir):
+        apply_fs(tmpdir, {'requirements.txt': ''})
+        build = self.get_build_config(
+            {
+                'version': 2,
+                'formats': ['pdf'],
+                'python': {
+                    'version': 3.6,
+                    'requirements': 'requirements.txt',
+                },
+            },
+            source_file=str(tmpdir.join('readthedocs.yml')),
+        )
+        build.validate()
+        expected_dict = {
+            'version': '2',
+            'formats': ['pdf'],
+            'python': {
+                'version': 3.6,
+                'requirements': str(tmpdir.join('requirements.txt')),
+                'install_with_pip': False,
+                'install_with_setup': False,
+                'extra_requirements': [],
+                'use_system_site_packages': False,
+            },
+            'build': {
+                'image': 'readthedocs/build:latest',
+            },
+            'conda': None,
+            'sphinx': {
+                'builder': 'sphinx',
+                'configuration': None,
+                'fail_on_warning': False,
+            },
+            'mkdocs': None,
+            'doctype': 'sphinx',
+            'submodules': {
+                'include': [],
+                'exclude': ALL,
+                'recursive': False,
+            },
+        }
+        assert build.as_dict() == expected_dict

readthedocs/projects/tasks.py

@@ -406,8 +406,10 @@ def run_setup(self, record=True):
                 raise YAMLParseError(
                     YAMLParseError.GENERIC_WITH_PARSE_EXCEPTION.format(
                         exception=str(e),
-                    ))
+                    )
+                )
 
+            self.save_build_config()
             self.additional_vcs_operations()
 
         if self.setup_env.failure or self.config is None:
@@ -531,9 +533,14 @@ def get_build(build_pk):
         build = {}
         if build_pk:
             build = api_v2.build(build_pk).get()
-        return dict((key, val) for (key, val) in list(build.items())
-                    if key not in ['project', 'version', 'resource_uri',
-                                   'absolute_uri'])
+        private_keys = [
+            'project', 'version', 'resource_uri', 'absolute_uri'
+        ]
+        return {
+            key: val
+            for key, val in build.items()
+            if key not in private_keys
+        }
 
     def setup_vcs(self):
         """
@@ -594,6 +601,15 @@ def set_valid_clone(self):
         self.project.has_valid_clone = True
         self.version.project.has_valid_clone = True
 
+    def save_build_config(self):
+        """Save config in the build object."""
+        pk = self.build['id']
+        config = self.config.as_dict()
+        api_v2.build(pk).patch({
+            'config': config,
+        })
+        self.build['config'] = config
+
     def update_app_instances(self, html=False, localmedia=False, search=False,
                              pdf=False, epub=False):
         """

readthedocs/restapi/serializers.py

@@ -112,18 +112,23 @@ class BuildSerializer(serializers.ModelSerializer):
     version_slug = serializers.ReadOnlyField(source='version.slug')
     docs_url = serializers.ReadOnlyField(source='version.get_absolute_url')
     state_display = serializers.ReadOnlyField(source='get_state_display')
+    # Jsonfield needs an explicit serializer
+    # https://github.com/dmkoch/django-jsonfield/issues/188#issuecomment-300439829
+    config = serializers.JSONField(required=False)
 
     class Meta(object):
         model = Build
-        exclude = ('builder',)
+        # `_config` should be excluded to avoid conflicts with `config`
+        exclude = ('builder', '_config')
 
 
 class BuildAdminSerializer(BuildSerializer):
 
     """Build serializer for display to admin users and build instances."""
 
     class Meta(BuildSerializer.Meta):
-        exclude = ()
+        # `_config` should be excluded to avoid conflicts with `config`
+        exclude = ('_config',)
 
 
 class SearchIndexSerializer(serializers.Serializer):