From e4d5e9b29d01f6c4a3cdad7c458d5df73a4cd741 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:07:49 -0700 Subject: [PATCH 01/17] Update models, managers and create migration for build commands This adds a build manager for the Build model, for moving everything to the v2 API --- .../migrations/0002_build_command_initial.py | 33 ++++++++++ readthedocs/builds/models.py | 66 +++++++++++++++++-- readthedocs/privacy/backend.py | 27 ++++++++ readthedocs/privacy/loader.py | 3 + readthedocs/projects/constants.py | 13 ++-- 5 files changed, 131 insertions(+), 11 deletions(-) create mode 100644 readthedocs/builds/migrations/0002_build_command_initial.py diff --git a/readthedocs/builds/migrations/0002_build_command_initial.py b/readthedocs/builds/migrations/0002_build_command_initial.py new file mode 100644 index 00000000000..b4b0f60cd53 --- /dev/null +++ b/readthedocs/builds/migrations/0002_build_command_initial.py @@ -0,0 +1,33 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations +import readthedocs.builds.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='BuildCommandResult', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('command', models.TextField(verbose_name='Command')), + ('description', models.TextField(null=True, verbose_name='Description', blank=True)), + ('output', models.TextField(null=True, verbose_name='Command output', blank=True)), + ('exit_code', models.IntegerField(default=0, verbose_name='Command exit code')), + ('start_time', models.DateTimeField(verbose_name='Start time')), + ('end_time', models.DateTimeField(verbose_name='End time')), + ('build', models.ForeignKey(related_name='commands', verbose_name='Build', to='builds.Build')), + ], + options={ + 'ordering': ['start_time'], + 'get_latest_by': 'start_time', + }, + bases=(readthedocs.builds.models.BuildCommandResultMixin, models.Model), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index df28489d4d2..5f99ce21eb3 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -11,13 +11,15 @@ from guardian.shortcuts import assign from taggit.managers import TaggableManager -from readthedocs.privacy.loader import VersionManager, RelatedProjectManager +from readthedocs.privacy.loader import (VersionManager, RelatedProjectManager, + RelatedBuildManager) from readthedocs.projects.models import Project -from readthedocs.projects import constants -from .constants import (BUILD_STATE, BUILD_TYPES, VERSION_TYPES, - LATEST, NON_REPOSITORY_VERSIONS, STABLE - ) +from readthedocs.projects.constants import (PRIVACY_CHOICES, REPO_TYPE_GIT, + REPO_TYPE_HG) +from .constants import (BUILD_STATE, BUILD_TYPES, VERSION_TYPES, + LATEST, NON_REPOSITORY_VERSIONS, STABLE, + BUILD_STATE_FINISHED) from .version_slug import VersionSlugField @@ -70,7 +72,7 @@ class Version(models.Model): built = models.BooleanField(_('Built'), default=False) uploaded = models.BooleanField(_('Uploaded'), default=False) privacy_level = models.CharField( - _('Privacy Level'), max_length=20, choices=constants.PRIVACY_CHOICES, + _('Privacy Level'), max_length=20, choices=PRIVACY_CHOICES, default=DEFAULT_VERSION_PRIVACY_LEVEL, help_text=_("Level of privacy for this Version.") ) tags = TaggableManager(blank=True) @@ -368,4 +370,54 @@ def get_absolute_url(self): @property def finished(self): '''Return if build has a finished state''' - return self.state == 'finished' + return self.state == BUILD_STATE_FINISHED + + +class BuildCommandResultMixin(object): + '''Mixin for common command result methods/properties + + Shared methods between the database model :py:cls:`BuildCommandResult` and + non-model respresentations of build command results from the API + ''' + + @property + def successful(self): + '''Did the command exit with a successful exit code''' + return self.exit_code == 0 + + @property + def failed(self): + '''Did the command exit with a failing exit code + + Helper for inverse of :py:meth:`successful`''' + return not self.successful + + +class BuildCommandResult(BuildCommandResultMixin, models.Model): + build = models.ForeignKey(Build, verbose_name=_('Build'), + related_name='commands') + + command = models.TextField(_('Command')) + description = models.TextField(_('Description'), null=True, blank=True) + output = models.TextField(_('Command output'), null=True, blank=True) + exit_code = models.IntegerField(_('Command exit code'), default=0) + + start_time = models.DateTimeField(_('Start time')) + end_time = models.DateTimeField(_('End time')) + + class Meta: + ordering = ['start_time'] + get_latest_by = 'start_time' + + objects = RelatedBuildManager() + + def __unicode__(self): + return (ugettext(u'Build command {pk} for build {build}') + .format(pk=self.pk, build=self.build)) + + @property + def run_time(self): + """Total command runtime in seconds""" + if self.start_time is not None and self.end_time is not None: + diff = self.end_time - self.start_time + return diff.seconds diff --git a/readthedocs/privacy/backend.py b/readthedocs/privacy/backend.py index bbf701b08c4..fdd8806024e 100644 --- a/readthedocs/privacy/backend.py +++ b/readthedocs/privacy/backend.py @@ -89,6 +89,33 @@ def api(self, user=None, *args, **kwargs): return self.public(user) +class RelatedBuildManager(models.Manager): + '''For models with association to a project through :py:cls:`Build`''' + + def _add_user_repos(self, queryset, user=None, *args, **kwargs): + # Hack around get_objects_for_user not supporting global perms + if user.has_perm('projects.view_project'): + return self.get_queryset().all().distinct() + if user.is_authenticated(): + # Add in possible user-specific views + project_qs = get_objects_for_user(user, 'projects.view_project') + pks = [p.pk for p in project_qs] + queryset = (self.get_queryset() + .filter(build__project__pk__in=pks) | queryset) + return queryset.distinct() + + def public(self, user=None, project=None, *args, **kwargs): + queryset = self.filter(build__project__privacy_level=constants.PUBLIC) + if user: + queryset = self._add_user_repos(queryset, user) + if project: + queryset = queryset.filter(build__project=project) + return queryset + + def api(self, user=None, *args, **kwargs): + return self.public(user) + + class VersionManager(RelatedProjectManager): def _add_user_repos(self, queryset, user=None, *args, **kwargs): diff --git a/readthedocs/privacy/loader.py b/readthedocs/privacy/loader.py index 0fc5d58008b..aa03747a76f 100644 --- a/readthedocs/privacy/loader.py +++ b/readthedocs/privacy/loader.py @@ -11,6 +11,9 @@ RelatedProjectManager = import_by_path( getattr(settings, 'RELATED_PROJECT_MANAGER', 'readthedocs.privacy.backend.RelatedProjectManager')) +RelatedBuildManager = import_by_path( + getattr(settings, 'RELATED_BUILD_MANAGER', + 'readthedocs.privacy.backend.RelatedBuildManager')) # Permissions AdminPermission = import_by_path( diff --git a/readthedocs/projects/constants.py b/readthedocs/projects/constants.py index 669cce8bac6..554fdf2e9c2 100644 --- a/readthedocs/projects/constants.py +++ b/readthedocs/projects/constants.py @@ -69,11 +69,16 @@ (DELETED_STATUS, _('Deleted')), ) +REPO_TYPE_GIT = 'git' +REPO_TYPE_SVN = 'svn' +REPO_TYPE_HG = 'hg' +REPO_TYPE_BZR = 'bzr' + REPO_CHOICES = ( - ('git', _('Git')), - ('svn', _('Subversion')), - ('hg', _('Mercurial')), - ('bzr', _('Bazaar')), + (REPO_TYPE_GIT, _('Git')), + (REPO_TYPE_SVN, _('Subversion')), + (REPO_TYPE_HG, _('Mercurial')), + (REPO_TYPE_BZR, _('Bazaar')), ) PUBLIC = 'public' From e4055d685388f084136774729c89377f0d9d6fc7 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:10:24 -0700 Subject: [PATCH 02/17] Changes to v2 API for build and build commands This also drops unused API endpoints and serializers --- readthedocs/restapi/permissions.py | 16 ++++++++++ readthedocs/restapi/serializers.py | 39 ++++++++++-------------- readthedocs/restapi/urls.py | 5 ++- readthedocs/restapi/views/model_views.py | 39 +++++++++++++++++++----- 4 files changed, 67 insertions(+), 32 deletions(-) diff --git a/readthedocs/restapi/permissions.py b/readthedocs/restapi/permissions.py index 855206bf0dd..e41b08684c7 100644 --- a/readthedocs/restapi/permissions.py +++ b/readthedocs/restapi/permissions.py @@ -43,3 +43,19 @@ def has_object_permission(self, request, view, obj): has_perm = super(APIPermission, self).has_object_permission( request, view, obj) return has_perm or (request.user and request.user.is_staff) + + +class APIRestrictedPermission(permissions.IsAdminUser): + """Allow admin write, authenticated and anonymous read only + + This differs from :py:cls:`APIPermission` by not allowing for authenticated + POSTs. This permission is endpoints like ``/api/v2/build/``, which are used + by admin users to coordinate build instance creation, but only should be + readable by end users. + """ + + def has_object_permission(self, request, view, obj): + return ( + request.method in permissions.SAFE_METHODS or + (request.user and request.user.is_staff) + ) diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index a97dc0918aa..4a095310b9f 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -1,6 +1,6 @@ from rest_framework import serializers -from readthedocs.builds.models import Build, Version +from readthedocs.builds.models import Build, BuildCommandResult, Version from readthedocs.projects.models import Project @@ -18,13 +18,6 @@ class Meta: ) -class ProjectFullSerializer(ProjectSerializer): - '''Serializer for all fields on project model''' - - class Meta: - model = Project - - class VersionSerializer(serializers.ModelSerializer): project = ProjectSerializer() downloads = serializers.DictField(source='get_downloads', read_only=True) @@ -40,29 +33,29 @@ class Meta: ) -class BuildSerializer(serializers.ModelSerializer): - project = ProjectSerializer() +class BuildCommandSerializer(serializers.ModelSerializer): + run_time = serializers.ReadOnlyField() class Meta: - model = Build - fields = ( - 'id', - 'project', - 'commit', - 'type', - 'date', - 'success', + model = BuildCommandResult - ) +class BuildSerializer(serializers.ModelSerializer): + """Writeable Build instance serializer, for admin access by builders""" -class VersionFullSerializer(VersionSerializer): - '''Serializer for all fields on version model''' + commands = BuildCommandSerializer(many=True, read_only=True) + state_display = serializers.ReadOnlyField(source='get_state_display') - project = ProjectFullSerializer() + class Meta: + model = Build + + +class BuildSerializerLimited(BuildSerializer): + """Readonly version of the build serializer, used for user facing display""" class Meta: - model = Version + model = Build + exclude = ('builder',) class SearchIndexSerializer(serializers.Serializer): diff --git a/readthedocs/restapi/urls.py b/readthedocs/restapi/urls.py index 0d18e6831e3..e69da967008 100644 --- a/readthedocs/restapi/urls.py +++ b/readthedocs/restapi/urls.py @@ -2,11 +2,14 @@ from rest_framework import routers -from .views.model_views import BuildViewSet, ProjectViewSet, NotificationViewSet, VersionViewSet +from .views.model_views import (BuildViewSet, BuildCommandViewSet, + ProjectViewSet, NotificationViewSet, + VersionViewSet) from readthedocs.comments.views import CommentViewSet router = routers.DefaultRouter() router.register(r'build', BuildViewSet) +router.register(r'command', BuildCommandViewSet) router.register(r'version', VersionViewSet) router.register(r'project', ProjectViewSet) router.register(r'notification', NotificationViewSet) diff --git a/readthedocs/restapi/views/model_views.py b/readthedocs/restapi/views/model_views.py index 1d083c1484f..a5be559cf59 100644 --- a/readthedocs/restapi/views/model_views.py +++ b/readthedocs/restapi/views/model_views.py @@ -8,17 +8,21 @@ from rest_framework.response import Response from readthedocs.builds.filters import VersionFilter -from readthedocs.builds.models import Build, Version +from readthedocs.builds.models import Build, BuildCommandResult, Version from readthedocs.core.utils import trigger_build from readthedocs.oauth import utils as oauth_utils from readthedocs.builds.constants import STABLE from readthedocs.projects.filters import ProjectFilter from readthedocs.projects.models import Project, EmailHook from readthedocs.projects.version_handling import determine_stable_version -from readthedocs.restapi.permissions import APIPermission -from readthedocs.restapi.permissions import RelatedProjectIsOwner -from readthedocs.restapi.serializers import BuildSerializer, ProjectSerializer, VersionSerializer -import readthedocs.restapi.utils as api_utils + +from ..permissions import (APIPermission, APIRestrictedPermission, + RelatedProjectIsOwner) +from ..serializers import (BuildSerializer, BuildSerializerLimited, + BuildCommandSerializer, ProjectSerializer, + VersionSerializer) +from .. import utils as api_utils + log = logging.getLogger(__name__) @@ -164,15 +168,34 @@ def downloads(self, request, **kwargs): }) -class BuildViewSet(viewsets.ReadOnlyModelViewSet): - permission_classes = [permissions.IsAuthenticatedOrReadOnly] +class BuildViewSet(viewsets.ModelViewSet): + permission_classes = [APIRestrictedPermission] renderer_classes = (JSONRenderer, BrowsableAPIRenderer) - serializer_class = BuildSerializer model = Build def get_queryset(self): return self.model.objects.api(self.request.user) + def get_serializer_class(self): + """Vary serializer class based on user status + + This is used to allow write to write-only fields on Build by admin users + and to not return those fields to non-admin users. + """ + if self.request.user.is_staff: + return BuildSerializer + return BuildSerializerLimited + + +class BuildCommandViewSet(viewsets.ModelViewSet): + permission_classes = [APIRestrictedPermission] + renderer_classes = [JSONRenderer, BrowsableAPIRenderer] + serializer_class = BuildCommandSerializer + model = BuildCommandResult + + def get_queryset(self): + return self.model.objects.api(self.request.user) + class NotificationViewSet(viewsets.ReadOnlyModelViewSet): permission_classes = (permissions.IsAuthenticated, RelatedProjectIsOwner) From 7a248a3a032e1a0711b2b1b67cdf64a3b179efa5 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:13:28 -0700 Subject: [PATCH 03/17] Use apiv2 in environments, track each command through buildenv Also makes command failures a warning class exception, so they are not reported as top level failures. Adds time tracking to start and end time to command execution. --- readthedocs/doc_builder/backends/sphinx.py | 6 +- readthedocs/doc_builder/environments.py | 108 +++++++++++++-------- 2 files changed, 71 insertions(+), 43 deletions(-) diff --git a/readthedocs/doc_builder/backends/sphinx.py b/readthedocs/doc_builder/backends/sphinx.py index 9814c235433..7af4b41c90f 100644 --- a/readthedocs/doc_builder/backends/sphinx.py +++ b/readthedocs/doc_builder/backends/sphinx.py @@ -269,10 +269,10 @@ def build(self, **kwargs): pdf_commands = [] for cmd in pdflatex_cmds: cmd_ret = self.run(*cmd, cwd=latex_cwd, warn_only=True) - # Force LaTeX status code to be a little more optimistic. If LaTeX + # Force LaTeX exit code to be a little more optimistic. If LaTeX # reports an output file, let's just assume we're fine. if PDF_RE.search(cmd_ret.output): - cmd_ret.status = 0 + cmd_ret.exit_code = 0 pdf_commands.append(cmd_ret) for cmd in makeindex_cmds: cmd_ret = self.run(*cmd, cwd=latex_cwd, warn_only=True) @@ -281,7 +281,7 @@ def build(self, **kwargs): cmd_ret = self.run(*cmd, cwd=latex_cwd, warn_only=True) pdf_match = PDF_RE.search(cmd_ret.output) if pdf_match: - cmd_ret.status = 0 + cmd_ret.exit_code = 0 self.pdf_file_name = pdf_match.group(1).strip() pdf_commands.append(cmd_ret) return all(cmd.successful for cmd in pdf_commands) diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index a97935d471c..a3b87565ea1 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -8,18 +8,22 @@ import logging import subprocess import traceback -import datetime import socket +from datetime import datetime from django.utils.text import slugify from django.utils.translation import ugettext_lazy as _ from docker import Client from docker.utils import create_host_config from docker.errors import APIError as DockerAPIError, DockerException +from rest_framework.renderers import JSONRenderer from readthedocs.builds.constants import BUILD_STATE_FINISHED +from readthedocs.builds.models import BuildCommandResultMixin from readthedocs.projects.constants import LOG_TEMPLATE from readthedocs.api.client import api as api_v1 +from readthedocs.restapi.client import api as api_v2 +from readthedocs.restapi.serializers import BuildCommandSerializer from .exceptions import (BuildEnvironmentException, BuildEnvironmentError, BuildEnvironmentWarning) @@ -30,22 +34,31 @@ log = logging.getLogger(__name__) -class BuildCommand(object): +class BuildCommand(BuildCommandResultMixin): '''Wrap command execution for execution in build environments This wraps subprocess commands with some logic to handle exceptions, logging, and setting up the env for the build command. + This acts a mapping of sorts to the API reprensentation of the + :py:cls:`readthedocs.builds.models.BuildCommandResult` model. + :param command: string or array of command parameters - :param cwd: current working path - :param shell: execute command in shell, default=True + :param cwd: current working path for the command + :param shell: execute command in shell, default=False :param environment: environment variables to add to environment + :type environment: dict + :param combine_output: combine stdout/stderr, default=True + :param input_data: data to pass in on stdin + :type input_data: str + :param build_env: build environment to use to execute commands + :param bin_path: binary path to add to PATH resolution + :param description: a more grokable description of the command being run ''' - # TODO add short name here for reporting def __init__(self, command, cwd=None, shell=False, environment=None, combine_output=True, input_data=None, build_env=None, - bin_path=None): + bin_path=None, description=None): self.command = command self.shell = shell if cwd is None: @@ -54,13 +67,18 @@ def __init__(self, command, cwd=None, shell=False, environment=None, self.environment = os.environ.copy() if environment is not None: self.environment.update(environment) + self.combine_output = combine_output - self.build_env = build_env - self.bin_path = bin_path - self.status = None self.input_data = input_data + self.build_env = build_env self.output = None self.error = None + self.start_time = None + self.end_time = None + + self.bin_path = bin_path + self.description = description + self.exit_code = None def __str__(self): # TODO do we want to expose the full command here? @@ -78,6 +96,7 @@ def run(self): ''' log.info("Running: '%s' [%s]", self.get_command(), self.cwd) + self.start_time = datetime.utcnow() stdout = subprocess.PIPE stderr = subprocess.PIPE stdin = None @@ -122,11 +141,13 @@ def run(self): self.error = cmd_stderr.decode('utf-8', 'replace') except (TypeError, AttributeError): self.error = None - self.status = proc.returncode + self.exit_code = proc.returncode except OSError: self.error = traceback.format_exc() self.output = self.error - self.status = -1 + self.exit_code = -1 + finally: + self.end_time = datetime.utcnow() def get_command(self): '''Flatten command''' @@ -135,17 +156,18 @@ def get_command(self): else: return self.command - @property - def successful(self): - '''Did the command exit with a successful exit code''' - return self.status == 0 - - @property - def failed(self): - '''Did the command exit with a failing exit code - - Helper for inverse of :py:meth:`successful`''' - return not self.successful + def save(self): + '''Save this command and result via the API''' + data = { + 'build': self.build_env.build.get('id'), + 'command': self.get_command(), + 'description': self.description, + 'output': self.output, + 'exit_code': self.exit_code, + 'start_time': self.start_time, + 'end_time': self.end_time, + } + api_v2.command.post(data) class DockerBuildCommand(BuildCommand): @@ -164,6 +186,7 @@ def run(self): log.info("Running in container %s: '%s' [%s]", self.build_env.container_id, self.get_command(), self.cwd) + self.start_time = datetime.utcnow() client = self.build_env.get_client() try: exec_cmd = client.exec_create( @@ -179,18 +202,20 @@ def run(self): except (TypeError, AttributeError): self.output = '' cmd_ret = client.exec_inspect(exec_id=exec_cmd['Id']) - self.status = cmd_ret['ExitCode'] + self.exit_code = cmd_ret['ExitCode'] # Docker will exit with a special exit code to signify the command # was killed due to memory usage, make the error code nicer. - if (self.status == DOCKER_OOM_EXIT_CODE and + if (self.exit_code == DOCKER_OOM_EXIT_CODE and self.output == 'Killed\n'): self.output = _('Command killed due to excessive memory ' 'consumption\n') except DockerAPIError: - self.status = -1 + self.exit_code = -1 if self.output is None or not self.output: self.output = _('Command exited abnormally') + finally: + self.end_time = datetime.utcnow() def get_wrapped_command(self): """Escape special bash characters in command to wrap in shell @@ -203,9 +228,13 @@ def get_wrapped_command(self): """ bash_escape_re = re.compile(r"([\t\ \!\"\#\$\&\'\(\)\*\:\;\<\>\?\@" r"\[\\\]\^\`\{\|\}\~])") - return ("/bin/sh -c 'cd {cwd} && {cmd}'" + prefix = '' + if self.bin_path: + prefix = 'PATH={0}:$PATH '.format(self.bin_path) + return ("/bin/sh -c 'cd {cwd} && {prefix}{cmd}'" .format( cwd=self.cwd, + prefix=prefix, cmd=(' '.join([bash_escape_re.sub(r'\\\1', part) for part in self.command])))) @@ -229,7 +258,7 @@ def __init__(self, project=None, version=None, build=None, record=True): self.record = record self.commands = [] self.failure = None - self.start_time = datetime.datetime.utcnow() + self.start_time = datetime.utcnow() def __enter__(self): return self @@ -278,6 +307,11 @@ def run(self, *cmd, **kwargs): cmd = self.command_class(cmd, **kwargs) self.commands.append(cmd) cmd.run() + + # Save to database + if self.record: + cmd.save() + if cmd.failed: msg = u'Command {cmd} failed'.format(cmd=cmd.get_command()) @@ -290,7 +324,7 @@ def run(self, *cmd, **kwargs): version=self.version.slug, msg=msg)) else: - raise BuildEnvironmentError(msg) + raise BuildEnvironmentWarning(msg) return cmd @property @@ -322,6 +356,8 @@ def update_build(self, state=None): if not self.record: return None + self.build['project'] = self.project.pk + self.build['version'] = self.version.pk self.build['builder'] = socket.gethostname() self.build['state'] = state if self.done: @@ -333,26 +369,18 @@ def update_build(self, state=None): BuildEnvironmentException): self.build['exit_code'] = self.failure.status_code elif len(self.commands) > 0: - self.build['exit_code'] = max([cmd.status + self.build['exit_code'] = max([cmd.exit_code for cmd in self.commands]) self.build['setup'] = self.build['setup_error'] = "" self.build['output'] = self.build['error'] = "" if self.start_time: - build_length = (datetime.datetime.utcnow() - self.start_time) + build_length = (datetime.utcnow() - self.start_time) self.build['length'] = build_length.total_seconds() - # TODO Replace this with per-command output tracking in the db - self.build['output'] = '\n'.join([str(cmd) - for cmd in self.commands - if cmd is not None]) - errors = [] if self.failure is not None: - errors.append(str(self.failure)) - errors.extend([str(cmd) for cmd in self.commands - if cmd is not None and cmd.failed]) - self.build['error'] = '\n'.join(errors) + self.build['error'] = str(self.failure) # Attempt to stop unicode errors on build reporting for key, val in self.build.items(): @@ -360,7 +388,7 @@ def update_build(self, state=None): self.build[key] = val.decode('utf-8', 'ignore') try: - api_v1.build(self.build['id']).put(self.build) + resp = api_v2.build(self.build['id']).put(self.build) except Exception: log.error("Unable to post a new build", exc_info=True) From bbc5e522be6e00f94301c2b1b9b46207e47a79b8 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:14:24 -0700 Subject: [PATCH 04/17] Add a better serializer to the apiv2 client --- readthedocs/restapi/client.py | 43 +++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/readthedocs/restapi/client.py b/readthedocs/restapi/client.py index 6b84fe3dcfe..65f7ddda8c3 100644 --- a/readthedocs/restapi/client.py +++ b/readthedocs/restapi/client.py @@ -1,7 +1,10 @@ -import slumber import logging +from slumber import API, serialize from django.conf import settings +from rest_framework.renderers import JSONRenderer +from rest_framework.parsers import JSONParser + log = logging.getLogger(__name__) @@ -9,9 +12,35 @@ PASS = getattr(settings, 'SLUMBER_PASSWORD', None) API_HOST = getattr(settings, 'SLUMBER_API_HOST', 'https://readthedocs.org') -if USER and PASS: - log.debug("Using slumber v2 with user %s, pointed at %s" % (USER, API_HOST)) - api = slumber.API(base_url='%s/api/v2/' % API_HOST, auth=(USER, PASS)) -else: - log.warning("SLUMBER_USERNAME/PASSWORD settings are not set") - api = slumber.API(base_url='%s/api/v2/' % API_HOST) + +class DrfJsonSerializer(serialize.JsonSerializer): + '''Additional serialization help from the DRF parser/renderer''' + key = 'json-drf' + + def loads(self, data): + return JSONParser().parse(data) + + def dumps(self, data): + return JSONRenderer().render(data) + + +def setup_api(): + api_config = { + 'base_url': '%s/api/v2/' % API_HOST, + 'serializer': serialize.Serializer( + default='json-drf', + serializers=[ + serialize.JsonSerializer(), + DrfJsonSerializer(), + ] + ) + } + if USER and PASS: + log.debug("Using slumber v2 with user %s, pointed at %s", USER, API_HOST) + api_config['auth'] = (USER, PASS) + else: + log.warning("SLUMBER_USERNAME/PASSWORD settings are not set") + return API(**api_config) + + +api = setup_api() From 365d8d92f411f9c2dfab1924f63ae5006d54a02c Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:14:55 -0700 Subject: [PATCH 05/17] Add serialized JSON build to build detail context --- readthedocs/builds/views.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index 087abc9b683..f49d4351178 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -1,17 +1,23 @@ +import logging + from django.shortcuts import get_object_or_404 from django.views.generic import ListView, DetailView from django.http import HttpResponsePermanentRedirect from django.conf import settings from django.core.urlresolvers import reverse - +from rest_framework.renderers import JSONRenderer from readthedocs.builds.models import Build, Version from readthedocs.builds.filters import BuildFilter from readthedocs.projects.models import Project +from readthedocs.restapi.serializers import BuildSerializer from redis import Redis, ConnectionError +log = logging.getLogger(__name__) + + class BuildList(ListView): model = Build @@ -63,6 +69,10 @@ def get_queryset(self): def get_context_data(self, **kwargs): context = super(BuildDetail, self).get_context_data(**kwargs) context['project'] = self.project + build_serializer = BuildSerializer(self.get_object()) + build_data = build_serializer.data + context['build_json'] = (JSONRenderer() + .render(build_data)) return context From 22db8f3d3dbd0e8620995fe872006366c561aa83 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:17:07 -0700 Subject: [PATCH 06/17] Add base javascript for build detail --- gulpfile.js | 1 + .../builds/static-src/builds/js/detail.js | 103 ++++++++++++++++++ readthedocs/builds/static/builds/js/detail.js | 1 + 3 files changed, 105 insertions(+) create mode 100644 readthedocs/builds/static-src/builds/js/detail.js create mode 100644 readthedocs/builds/static/builds/js/detail.js diff --git a/gulpfile.js b/gulpfile.js index 8964d6bdb77..60ad029d06f 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -17,6 +17,7 @@ var gulp = require('gulp'), // picking up dependencies of the primary entry points and putting any // limitations on directory structure for entry points. var sources = { + builds: ['js/detail.js'], core: [ 'js/readthedocs-doc-embed.js', 'js/autocomplete.js', diff --git a/readthedocs/builds/static-src/builds/js/detail.js b/readthedocs/builds/static-src/builds/js/detail.js new file mode 100644 index 00000000000..ff00fb64e41 --- /dev/null +++ b/readthedocs/builds/static-src/builds/js/detail.js @@ -0,0 +1,103 @@ +// Build detail view + +var ko = window.knockout || require('knockout'); +var $ = window.jquery || require('jquery'); + + +function BuildCommand (data) { + var self = this; + self.id = ko.observable(data.id); + self.command = ko.observable(data.command); + self.output = ko.observable(data.output); + self.exit_code = ko.observable(data.exit_code || 0); + self.successful = ko.observable(self.exit_code() == 0); + self.run_time = ko.observable(data.run_time); + self.is_showing = ko.observable(!self.successful()); + + self.toggleCommand = function () { + self.is_showing(!self.is_showing()); + }; + + self.command_status = ko.computed(function () { + return self.successful() ? + 'build-command-successful' : + 'build-command-failed'; + }); +} + +function BuildDetailView (instance) { + var self = this, + instance = instance || {}; + + /* Instance variables */ + self.state = ko.observable(instance.state); + self.state_display = ko.observable(instance.state_display); + self.finished = ko.computed(function () { + return self.state() == 'finished'; + }); + self.date = ko.observable(instance.date); + self.success = ko.observable(instance.success); + self.error = ko.observable(instance.error); + self.length = ko.observable(instance.length); + self.commands = ko.observableArray(instance.commands); + self.display_commands = ko.computed(function () { + var commands_display = [], + commands_raw = self.commands(); + for (n in commands_raw) { + var command = new BuildCommand(commands_raw[n]); + commands_display.push(command) + } + return commands_display; + }); + self.commit = ko.observable(instance.commit); + + /* Others */ + self.legacy_output = ko.observable(false); + self.show_legacy_output = function () { + self.legacy_output(true); + }; + + function poll_api () { + if (self.finished()) { + return; + } + $.getJSON('/api/v2/build/' + instance.id + '/', function (data) { + self.state(data.state); + self.state_display(data.state_display); + self.date(data.date); + self.success(data.success); + self.error(data.error); + self.length(data.length); + self.commit(data.commit); + for (n in data.commands) { + var command = data.commands[n]; + var match = ko.utils.arrayFirst( + self.commands(), + function(command_cmp) { + return (command_cmp.id == command.id); + } + ); + if (!match) { + self.commands.push(command); + } + } + }); + + setTimeout(poll_api, 2000); + } + + poll_api(); +} + +BuildDetailView.init = function (instance, domobj) { + var view = new BuildDetailView(instance), + domobj = domobj || $('#build-detail')[0]; + ko.applyBindings(view, domobj); + return view; +}; + +module.exports.BuildDetailView = BuildDetailView; + +if (typeof(window) != 'undefined') { + window.build = module.exports; +} diff --git a/readthedocs/builds/static/builds/js/detail.js b/readthedocs/builds/static/builds/js/detail.js new file mode 100644 index 00000000000..b6583574fe2 --- /dev/null +++ b/readthedocs/builds/static/builds/js/detail.js @@ -0,0 +1 @@ +!function e(t,o,n){function r(i,u){if(!o[i]){if(!t[i]){var a="function"==typeof require&&require;if(!u&&a)return a(i,!0);if(s)return s(i,!0);var c=new Error("Cannot find module '"+i+"'");throw c.code="MODULE_NOT_FOUND",c}var d=o[i]={exports:{}};t[i][0].call(d.exports,function(e){var o=t[i][1][e];return r(o?o:e)},d,d.exports,e,t,o,n)}return o[i].exports}for(var s="function"==typeof require&&require,i=0;i Date: Fri, 14 Aug 2015 16:18:08 -0700 Subject: [PATCH 07/17] Fix tests --- readthedocs/rtd_tests/tests/test_api.py | 142 +++++++++++++++--- .../rtd_tests/tests/test_doc_building.py | 12 +- 2 files changed, 131 insertions(+), 23 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_api.py b/readthedocs/rtd_tests/tests/test_api.py index e341d815bd0..cecf5dcd6bf 100644 --- a/readthedocs/rtd_tests/tests/test_api.py +++ b/readthedocs/rtd_tests/tests/test_api.py @@ -1,6 +1,15 @@ -from django.test import TestCase import json import base64 +import datetime +import unittest + +from django.test import TestCase +from django.contrib.auth.models import User +from django_dynamic_fixture import get +from rest_framework import status +from rest_framework.test import APIClient + +from readthedocs.builds.models import Build super_auth = base64.b64encode('super:test') @@ -14,24 +23,121 @@ def test_make_build(self): """ Test that a superuser can use the API """ - post_data = { - "project": "/api/v1/project/1/", - "version": "/api/v1/version/1/", - "success": True, - "output": "Test Output", - "error": "Test Error", - } - resp = self.client.post('/api/v1/build/', data=json.dumps(post_data), - content_type='application/json', - HTTP_AUTHORIZATION='Basic %s' % super_auth) - self.assertEqual(resp.status_code, 201) - self.assertEqual(resp['location'], - 'http://testserver/api/v1/build/1/') - resp = self.client.get('/api/v1/build/1/', data={'format': 'json'}, - HTTP_AUTHORIZATION='Basic %s' % super_auth) + client = APIClient() + client.login(username='super', password='test') + resp = client.post( + '/api/v2/build/', + { + 'project': 1, + 'version': 1, + 'success': True, + 'output': 'Test Output', + 'error': 'Test Error', + 'state': 'cloning', + }, + format='json') + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + build = resp.data + self.assertEqual(build['id'], 1) + self.assertEqual(build['state_display'], 'Cloning') + + resp = client.get('/api/v2/build/1/') self.assertEqual(resp.status_code, 200) - obj = json.loads(resp.content) - self.assertEqual(obj['output'], 'Test Output') + build = resp.data + self.assertEqual(build['output'], 'Test Output') + self.assertEqual(build['state_display'], 'Cloning') + + def test_make_build_without_permission(self): + """Ensure anonymous/non-staff users cannot write the build endpoint""" + client = APIClient() + + def _try_post(): + resp = client.post( + '/api/v2/build/', + { + 'project': 1, + 'version': 1, + 'success': True, + 'output': 'Test Output', + 'error': 'Test Error', + }, + format='json') + self.assertEqual(resp.status_code, 403) + + _try_post() + + api_user = get(User, staff=False, password='test') + assert api_user.is_staff == False + client.force_authenticate(user=api_user) + _try_post() + + def test_update_build_without_permission(self): + """Ensure anonymous/non-staff users cannot update build endpoints""" + client = APIClient() + api_user = get(User, staff=False, password='test') + client.force_authenticate(user=api_user) + build = get(Build, project_id=1, version_id=1, state='cloning') + resp = client.put( + '/api/v2/build/{0}/'.format(build.pk), + { + 'project': 1, + 'version': 1, + 'state': 'finished' + }, + format='json') + self.assertEqual(resp.status_code, 403) + + def test_make_build_protected_fields(self): + """Ensure build api view delegates correct serializer + + Super users should be able to read/write the `builder` property, but we + don't expose this to end users via the API + """ + build = get(Build, project_id=1, version_id=1, builder='foo') + client = APIClient() + + api_user = get(User, staff=False, password='test') + client.force_authenticate(user=api_user) + resp = client.get('/api/v2/build/{0}/'.format(build.pk), format='json') + self.assertEqual(resp.status_code, 403) + + client.force_authenticate(user=User.objects.get(username='super')) + resp = client.get('/api/v2/build/{0}/'.format(build.pk), format='json') + self.assertEqual(resp.status_code, 200) + self.assertIn('builder', resp.data) + + def test_make_build_commands(self): + """Create build and build commands""" + client = APIClient() + client.login(username='super', password='test') + resp = client.post( + '/api/v2/build/', + { + 'project': 1, + 'version': 1, + 'success': True, + }, + format='json') + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + build = resp.data + now = datetime.datetime.utcnow() + resp = client.post( + '/api/v2/command/', + { + 'build': build['id'], + 'command': 'echo test', + 'description': 'foo', + 'start_time': str(now - datetime.timedelta(seconds=5)), + 'end_time': str(now), + }, + format='json') + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + resp = client.get('/api/v2/build/1/') + self.assertEqual(resp.status_code, 200) + build = resp.data + self.assertEqual(len(build['commands']), 1) + self.assertEqual(build['commands'][0]['run_time'], 5) + self.assertEqual(build['commands'][0]['description'], 'foo') class APITests(TestCase): diff --git a/readthedocs/rtd_tests/tests/test_doc_building.py b/readthedocs/rtd_tests/tests/test_doc_building.py index 65132536918..e6619cc60c3 100644 --- a/readthedocs/rtd_tests/tests/test_doc_building.py +++ b/readthedocs/rtd_tests/tests/test_doc_building.py @@ -173,7 +173,7 @@ def test_command_execution(self): stderr=True, stdout=True ) - self.assertEqual(build_env.commands[0].status, 1) + self.assertEqual(build_env.commands[0].exit_code, 1) self.assertEqual(build_env.commands[0].output, 'This is the return') self.assertEqual(build_env.commands[0].error, None) self.assertTrue(build_env.failed) @@ -345,13 +345,15 @@ def test_wrapped_command(self): ("/bin/sh -c " "'cd /tmp/foobar && " "pip install requests'")) - cmd = DockerBuildCommand(['pip', 'install', 'Django>1.7'], - cwd='/tmp/foobar') + cmd = DockerBuildCommand(['python', '/tmp/foo/pip', 'install', + 'Django>1.7'], + cwd='/tmp/foobar', + bin_path='/tmp/foo') self.assertEqual( cmd.get_wrapped_command(), ("/bin/sh -c " - "'cd /tmp/foobar && " - "pip install Django\>1.7'")) + "'cd /tmp/foobar && PATH=/tmp/foo:$PATH " + "python /tmp/foo/pip install Django\>1.7'")) def test_unicode_output(self): '''Unicode output from command''' From a95f08d9b16aaae57c51e47434185a4af83ad72d Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:18:30 -0700 Subject: [PATCH 08/17] HTML and CSS for build detail page --- .../builds/static/builds/css/detail.css | 143 +++++++++++ .../templates/builds/build_detail.html | 225 ++++++++++++------ 2 files changed, 290 insertions(+), 78 deletions(-) create mode 100644 readthedocs/builds/static/builds/css/detail.css diff --git a/readthedocs/builds/static/builds/css/detail.css b/readthedocs/builds/static/builds/css/detail.css new file mode 100644 index 00000000000..9d7550fb185 --- /dev/null +++ b/readthedocs/builds/static/builds/css/detail.css @@ -0,0 +1,143 @@ +/* Build detail styles */ + +div.build-detail div.build-id { + font-size: 1.5em; +} + +div.build-detail div.build-version { + line-height: 2em; +} + +div.build-detail div.build-version span.build-commit { + color: #887; + font-style: italic; +} + +div.build-detail div.build-state { + margin: .7em 0em 1em 0em; + font-size: 1.2em; +} + +div.build-detail div.build-state > span.build-state-successful, +div.build-detail div.build-state > span.build-state-failed { + padding: .3em .5em; + border-radius: .3em; + -moz-border-radius: .3em; + -ms-border-radius: .3em; + -webkit-border-radius: .3em; + color: white; +} + +div.build-detail div.build-state > span.build-state-failed { + background: #a55; +} +div.build-detail div.build-state > span.build-state-successful { + background: #5a5; +} + +ul.build-meta { + display: block; + width: 40%; + float: right; + overflow: auto; + font-size: .9em; + text-align: right; +} + +/* Error box */ +div.build-detail div.build-error { + padding: .3em .5em; + margin: .3em; + border: 2px solid #a55; + border-radius: .2em; + -moz-border-radius: .2em; + -ms-border-radius: .2em; + -webkit-border-radius: .2em; + color: #a55; +} + +div.build-detail div.build-error h3 { + text-shadow: none; + font-weight: normal; + color: #a55; +} + +div.build-detail div.build-error p { + margin-bottom: 0em; +} + +/* Build command list */ +div#build-commands { + clear: right; +} + +div.build-command { + padding: .5em; + margin: 5px; + font-size: .9em; + background: #eee; + color: #443; + border: 1px solid #e4e4e4; + border-bottom-color: #ddd; + + border-radius: .3em; + -moz-border-radius: .3em; + -webkit-border-radius: .3em; + -ms-border-radius: .3em; +} + +div.build-command.build-command-failed { + background: #a55; + color: white; +} + +div.build-command div.build-command-run, +div.build-command div.build-command-output { + display: block; + overflow: hidden; +} + +div.build-command div.build-command-run { + cursor: pointer; +} + +div.build-command div.build-command-output { + margin-top: .5em; +} + +div.build-command div.build-command-run > span, +div.build-command div.build-command-output > span { + display: block; + margin-bottom: -16px; + font-family: Consolas, "Liberation Mono", Menlo, Courier, monospace; + white-space: pre; + overflow-x: scroll; + overflow-y: hidden; +} + +div.build-command div.build-command-run > span::-webkit-scrollbar, +div.build-command div.build-command-output > span::-webkit-scrollbar { + -webkit-appearance: none; + height: 15px; +} + +div.build-command div.build-command-output > span { + padding: .5em; +} + +div.build-command div.build-command-output, +div.build-command div.build-command-meta { + background: #443; + color: #ffe; +} + +div.build-command div.build-command-meta { + padding: .5em; + font-family: Consolas, "Liberation Mono", Menlo, Courier, monospace; + text-align: right; + color: #bba; +} + +div.build-command.build-command-failed div.build-command-meta { + color: #a55; +} diff --git a/readthedocs/templates/builds/build_detail.html b/readthedocs/templates/builds/build_detail.html index c555177e6f4..080786bb7f9 100644 --- a/readthedocs/templates/builds/build_detail.html +++ b/readthedocs/templates/builds/build_detail.html @@ -1,99 +1,168 @@ {% extends "projects/base_project.html" %} {% load i18n %} +{% load static %} {% block title %}{{ build.project.name }}{% endblock %} +{% block extra_scripts %} + + + +{% endblock %} -{% block project_editing %} - {% with builds_active="active" %} - {% include "core/project_bar.html" %} - {% endwith %} +{% block extra_links %} + {% endblock %} -{% block content-header %}

{% blocktrans with build.project.name as project_name %}Build for {{ project_name }}{% endblocktrans %}

{% endblock %} +{% block project_editing %} + {% with builds_active="active" %} + {% include "core/project_bar.html" %} + {% endwith %} +{% endblock %} {% block content %} -
-

{% blocktrans with build.date|date:"N j, Y. P" as build_date %}Built: {{ build_date }}{% endblocktrans %}

- -

{% trans "State:" %} {{ build.get_state_display }} - -

- -

{% trans "Outcome:" %} - - - {% if build.state != 'finished' %}{% trans "Not yet finished" %} {% else %} {% if build.success %}{% trans "Passed" %}{% else %}{% trans "Failed" %}{% endif %}{% endif %} {% if not build.success %} ({% trans "Status Code:" %} {{ build.exit_code }}){% endif %} - - -

- - {% if build.version %} -

{% trans "Version:" %} {{ build.version.slug }}

- {% endif %} - -

{% trans "Type:" %} {{ build.type }}

- - {% if build.commit %} -

{% trans "Commit" %}: {{ build.commit }}

- {% endif %} - - {% if build.length %} -

{% trans "Length:" %} {{ build.length }} seconds

- {% endif %} - - {% if build.builder and request.user.is_staff %} -

{% trans "Builder:" %} {{ build.builder }}

- {% endif %} +
+ + +
    +
    +
  • + {% trans "Completed" %} + {{ build.date|date:"N j, Y. P" }} +
    • + +
  • + {% trans "Build took" %} + + {{ build.length }} + + {% trans "seconds" %} +
    • +
    + + {% if build.builder and request.user.is_staff %} +
  • + {% trans "Built on" %} + {{ build.builder }} +
    • + {% endif %} +
-

- -

+
+ {% blocktrans with build_id=build.pk %} + Build #{{ build_id }} + {% endblocktrans %} +
- - {% if build.error %} -

{% trans "Build Errors" %}

- 
{{ build.error }}
+ {% else %} + {# Show new command output if lacking build.output #} + + + + {% endif %}
- -{% endblock %} - -{% block extra_scripts %} -{{ block.super }} - - - {% endblock %} From 3957137f69ac5a8f46d915014e8be2d885f7a31e Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 15:51:21 -0700 Subject: [PATCH 09/17] Drop APIv1 builds --- readthedocs/api/base.py | 35 ----------------------------------- readthedocs/urls.py | 3 +-- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/readthedocs/api/base.py b/readthedocs/api/base.py index d222e1c612c..e5e71d5b768 100644 --- a/readthedocs/api/base.py +++ b/readthedocs/api/base.py @@ -189,41 +189,6 @@ def override_urls(self): ] -class BuildResource(ModelResource): - project = fields.ForeignKey('readthedocs.api.base.ProjectResource', 'project') - version = fields.ForeignKey('readthedocs.api.base.VersionResource', 'version') - - class Meta(object): - always_return_data = True - include_absolute_url = True - allowed_methods = ['get', 'post', 'put'] - queryset = Build.objects.api() - authentication = PostAuthentication() - authorization = DjangoAuthorization() - filtering = { - "project": ALL_WITH_RELATIONS, - "slug": ALL_WITH_RELATIONS, - "type": ALL_WITH_RELATIONS, - "state": ALL_WITH_RELATIONS, - } - - def get_object_list(self, request): - self._meta.queryset = Build.objects.api(user=request.user) - return super(BuildResource, self).get_object_list(request) - - def override_urls(self): - return [ - url(r"^(?P%s)/schema/$" - % self._meta.resource_name, - self.wrap_view('get_schema'), - name="api_get_schema"), - url(r"^(?P%s)/(?P[a-z-_]+)/$" % - self._meta.resource_name, - self.wrap_view('dispatch_list'), - name="build_list_detail"), - ] - - class FileResource(ModelResource, SearchMixin): project = fields.ForeignKey(ProjectResource, 'project', full=True) diff --git a/readthedocs/urls.py b/readthedocs/urls.py index 1eecd3ed1e0..25307457220 100644 --- a/readthedocs/urls.py +++ b/readthedocs/urls.py @@ -5,14 +5,13 @@ from tastypie.api import Api -from readthedocs.api.base import (ProjectResource, UserResource, BuildResource, +from readthedocs.api.base import (ProjectResource, UserResource, VersionResource, FileResource) from readthedocs.core.views import HomepageView from readthedocs.core.urls import docs_urls, core_urls, deprecated_urls v1_api = Api(api_name='v1') -v1_api.register(BuildResource()) v1_api.register(UserResource()) v1_api.register(ProjectResource()) v1_api.register(VersionResource()) From c25da5526f5b997132b3f25800e788f622a728a5 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:26:50 -0700 Subject: [PATCH 10/17] Drop missed apiv1 build call --- readthedocs/projects/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 01f58245630..7da274ba7c9 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -173,7 +173,7 @@ def get_build(build_pk): """ build = {} if build_pk: - build = api_v1.build(build_pk).get() + build = api_v2.build(build_pk).get() return dict((key, val) for (key, val) in build.items() if key not in ['project', 'version', 'resource_uri', 'absolute_uri']) From 9abe81d0472d7b3237e1563da9637b9d9bcf14c9 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 14 Aug 2015 16:27:34 -0700 Subject: [PATCH 11/17] Fix lint issue with model views --- readthedocs/restapi/views/model_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/readthedocs/restapi/views/model_views.py b/readthedocs/restapi/views/model_views.py index a5be559cf59..d8defe511ea 100644 --- a/readthedocs/restapi/views/model_views.py +++ b/readthedocs/restapi/views/model_views.py @@ -19,8 +19,8 @@ from ..permissions import (APIPermission, APIRestrictedPermission, RelatedProjectIsOwner) from ..serializers import (BuildSerializer, BuildSerializerLimited, - BuildCommandSerializer, ProjectSerializer, - VersionSerializer) + BuildCommandSerializer, ProjectSerializer, + VersionSerializer) from .. import utils as api_utils log = logging.getLogger(__name__) From 32fa423caa9dbb5c6654921a2115fb1bb1fc5dc7 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Tue, 18 Aug 2015 23:53:38 -0700 Subject: [PATCH 12/17] Swap serializer inheritance --- readthedocs/builds/views.py | 4 ++-- readthedocs/restapi/serializers.py | 8 ++++---- readthedocs/restapi/views/model_views.py | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index f49d4351178..2c6188d174c 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -10,7 +10,7 @@ from readthedocs.builds.models import Build, Version from readthedocs.builds.filters import BuildFilter from readthedocs.projects.models import Project -from readthedocs.restapi.serializers import BuildSerializer +from readthedocs.restapi.serializers import BuildSerializerFull from redis import Redis, ConnectionError @@ -69,7 +69,7 @@ def get_queryset(self): def get_context_data(self, **kwargs): context = super(BuildDetail, self).get_context_data(**kwargs) context['project'] = self.project - build_serializer = BuildSerializer(self.get_object()) + build_serializer = BuildSerializerFull(self.get_object()) build_data = build_serializer.data context['build_json'] = (JSONRenderer() .render(build_data)) diff --git a/readthedocs/restapi/serializers.py b/readthedocs/restapi/serializers.py index 4a095310b9f..0fe06223a72 100644 --- a/readthedocs/restapi/serializers.py +++ b/readthedocs/restapi/serializers.py @@ -41,21 +41,21 @@ class Meta: class BuildSerializer(serializers.ModelSerializer): - """Writeable Build instance serializer, for admin access by builders""" + """Readonly version of the build serializer, used for user facing display""" commands = BuildCommandSerializer(many=True, read_only=True) state_display = serializers.ReadOnlyField(source='get_state_display') class Meta: model = Build + exclude = ('builder',) -class BuildSerializerLimited(BuildSerializer): - """Readonly version of the build serializer, used for user facing display""" +class BuildSerializerFull(BuildSerializer): + """Writeable Build instance serializer, for admin access by builders""" class Meta: model = Build - exclude = ('builder',) class SearchIndexSerializer(serializers.Serializer): diff --git a/readthedocs/restapi/views/model_views.py b/readthedocs/restapi/views/model_views.py index d8defe511ea..711ee386786 100644 --- a/readthedocs/restapi/views/model_views.py +++ b/readthedocs/restapi/views/model_views.py @@ -18,7 +18,7 @@ from ..permissions import (APIPermission, APIRestrictedPermission, RelatedProjectIsOwner) -from ..serializers import (BuildSerializer, BuildSerializerLimited, +from ..serializers import (BuildSerializerFull, BuildSerializer, BuildCommandSerializer, ProjectSerializer, VersionSerializer) from .. import utils as api_utils @@ -183,8 +183,8 @@ def get_serializer_class(self): and to not return those fields to non-admin users. """ if self.request.user.is_staff: - return BuildSerializer - return BuildSerializerLimited + return BuildSerializerFull + return BuildSerializer class BuildCommandViewSet(viewsets.ModelViewSet): From 52a1c3f23648510f34531243ec8282158c86f7e9 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Tue, 18 Aug 2015 23:56:16 -0700 Subject: [PATCH 13/17] Drop styling on build command id selector --- readthedocs/builds/static/builds/css/detail.css | 2 +- readthedocs/templates/builds/build_detail.html | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/builds/static/builds/css/detail.css b/readthedocs/builds/static/builds/css/detail.css index 9d7550fb185..8731fc9e9b3 100644 --- a/readthedocs/builds/static/builds/css/detail.css +++ b/readthedocs/builds/static/builds/css/detail.css @@ -67,7 +67,7 @@ div.build-detail div.build-error p { } /* Build command list */ -div#build-commands { +div.build-command-list { clear: right; } diff --git a/readthedocs/templates/builds/build_detail.html b/readthedocs/templates/builds/build_detail.html index 080786bb7f9..df6d4b92705 100644 --- a/readthedocs/templates/builds/build_detail.html +++ b/readthedocs/templates/builds/build_detail.html @@ -134,6 +134,7 @@

{% trans "Error" %}