diff --git a/readthedocs/builds/migrations/0007_make-build-command-result-attrs-nullable.py b/readthedocs/builds/migrations/0007_make-build-command-result-attrs-nullable.py new file mode 100644 index 00000000000..564c742b31b --- /dev/null +++ b/readthedocs/builds/migrations/0007_make-build-command-result-attrs-nullable.py @@ -0,0 +1,25 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.16 on 2018-12-19 21:49 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0006_add_config_field'), + ] + + operations = [ + migrations.AlterField( + model_name='buildcommandresult', + name='end_time', + field=models.DateTimeField(blank=True, default=None, null=True, verbose_name='End time'), + ), + migrations.AlterField( + model_name='buildcommandresult', + name='exit_code', + field=models.IntegerField(blank=True, default=None, null=True, verbose_name='Command exit code'), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 93678975232..ccd84025564 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -606,10 +606,14 @@ class BuildCommandResult(BuildCommandResultMixin, models.Model): command = models.TextField(_('Command')) description = models.TextField(_('Description'), blank=True) output = models.TextField(_('Command output'), blank=True) - exit_code = models.IntegerField(_('Command exit code')) + exit_code = models.IntegerField( + _('Command exit code'), null=True, blank=True, default=None + ) start_time = models.DateTimeField(_('Start time')) - end_time = models.DateTimeField(_('End time')) + end_time = models.DateTimeField( + _('End time'), null=True, blank=True, default=None + ) class Meta(object): ordering = ['start_time'] @@ -628,3 +632,4 @@ def run_time(self): if self.start_time is not None and self.end_time is not None: diff = self.end_time - self.start_time return diff.seconds + return None diff --git a/readthedocs/builds/static-src/builds/js/detail.js b/readthedocs/builds/static-src/builds/js/detail.js index 58637d6b1ee..2df3e206399 100644 --- a/readthedocs/builds/static-src/builds/js/detail.js +++ b/readthedocs/builds/static-src/builds/js/detail.js @@ -9,8 +9,8 @@ function BuildCommand(data) { 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.exit_code = ko.observable(data.exit_code); + self.successful = ko.observable(self.exit_code() === null || self.exit_code() === 0); self.run_time = ko.observable(data.run_time); self.is_showing = ko.observable(!self.successful()); @@ -23,6 +23,10 @@ function BuildCommand(data) { 'build-command-successful' : 'build-command-failed'; }); + + self.is_running = ko.computed(function () { + return self.exit_code() !== null && self.run_time() != null; + }); } function BuildDetailView(instance) { @@ -83,6 +87,10 @@ function BuildDetailView(instance) { ); if (!match) { self.commands.push(command); + } else { + match.output = command.output; + match.exit_code = command.exit_code; + match.run_time = command.run_time; } } }); diff --git a/readthedocs/doc_builder/environments.py b/readthedocs/doc_builder/environments.py index d48fc73641f..f1b31bda44c 100644 --- a/readthedocs/doc_builder/environments.py +++ b/readthedocs/doc_builder/environments.py @@ -97,6 +97,7 @@ class BuildCommand(BuildCommandResultMixin): def __init__(self, command, cwd=None, shell=False, environment=None, combine_output=True, input_data=None, build_env=None, bin_path=None, description=None, record_as_success=False): + self.command_id = None self.command = command self.shell = shell if cwd is None: @@ -249,6 +250,18 @@ def get_command(self): def save(self): """Save this command and result via the API.""" + self.start_time = self.start_time or datetime.utcnow() + data = { + 'build': self.build_env.build.get('id'), + 'command': self.get_command(), + 'description': self.description, + 'start_time': self.start_time, + } + resp = api_v2.command.post(data) + self.command_id = resp['id'] + + def update(self): + """Update this command and result via the API.""" # Force record this command as success to avoid Build reporting errors # on commands that are just for checking purposes and do not interferes # in the Build @@ -257,15 +270,11 @@ def save(self): self.exit_code = 0 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) + api_v2.command(self.command_id).patch(data) class DockerBuildCommand(BuildCommand): @@ -363,6 +372,9 @@ def __init__(self, project, environment=None): def record_command(self, command): pass + def update_command(self, command): + pass + def run(self, *cmd, **kwargs): """Shortcut to run command from environment.""" return self.run_command_class(cls=self.command_class, cmd=cmd, **kwargs) @@ -404,13 +416,15 @@ def run_command_class( # ``build_env`` is passed as ``kwargs`` when it's called from a # ``*BuildEnvironment`` build_cmd = cls(cmd, **kwargs) + if record: + self.record_command(build_cmd) build_cmd.run() if record: # TODO: I don't like how it's handled this entry point here since # this class should know nothing about a BuildCommand (which are the # only ones that can be saved/recorded) - self.record_command(build_cmd) + self.update_command(build_cmd) # We want append this command to the list of commands only if it has # to be recorded in the database (to keep consistency) and also, it @@ -559,6 +573,9 @@ def handle_exception(self, exc_type, exc_value, _): def record_command(self, command): command.save() + def update_command(self, command): + command.update() + def run(self, *cmd, **kwargs): kwargs.update({ 'build_env': self, diff --git a/readthedocs/rtd_tests/tests/test_builds.py b/readthedocs/rtd_tests/tests/test_builds.py index 53e222203fe..e4c1eaf0bdf 100644 --- a/readthedocs/rtd_tests/tests/test_builds.py +++ b/readthedocs/rtd_tests/tests/test_builds.py @@ -44,6 +44,10 @@ def test_build(self, load_config): self.mocks.configure_mock('api', { 'get.return_value': {'downloads': "no_url_here"} }) + self.mocks.configure_mock('api_versions', {'return_value': [version]}) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) self.mocks.patches['html_build'].stop() build_env = LocalBuildEnvironment(project=project, version=version, build={}) @@ -188,6 +192,9 @@ def test_build_pdf_latex_failures(self, load_config): load_config.side_effect = create_load() self.mocks.patches['html_build'].stop() self.mocks.patches['pdf_build'].stop() + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) project = get(Project, slug='project-1', @@ -234,6 +241,9 @@ def test_build_pdf_latex_not_failure(self, load_config): load_config.side_effect = create_load() self.mocks.patches['html_build'].stop() self.mocks.patches['pdf_build'].stop() + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) project = get(Project, slug='project-2', diff --git a/readthedocs/rtd_tests/tests/test_doc_building.py b/readthedocs/rtd_tests/tests/test_doc_building.py index 6a954de74d6..46a4d30d6d4 100644 --- a/readthedocs/rtd_tests/tests/test_doc_building.py +++ b/readthedocs/rtd_tests/tests/test_doc_building.py @@ -69,6 +69,9 @@ def test_normal_execution(self): self.mocks.configure_mock('process', { 'communicate.return_value': (b'This is okay', '') }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) type(self.mocks.process).returncode = PropertyMock(return_value=0) build_env = LocalBuildEnvironment( @@ -87,15 +90,17 @@ def test_normal_execution(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 0, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -155,6 +160,9 @@ def test_record_command_as_success(self): self.mocks.configure_mock('process', { 'communicate.return_value': (b'This is okay', '') }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) type(self.mocks.process).returncode = PropertyMock(return_value=1) build_env = LocalBuildEnvironment( @@ -173,15 +181,17 @@ def test_record_command_as_success(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 0, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -239,6 +249,9 @@ def test_failing_execution(self): self.mocks.configure_mock('process', { 'communicate.return_value': (b'This is not okay', '') }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) type(self.mocks.process).returncode = PropertyMock(return_value=1) build_env = LocalBuildEnvironment( @@ -258,15 +271,17 @@ def test_failing_execution(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 1, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -556,6 +571,9 @@ def test_api_failure_on_docker_memory_limit(self): 'Failure creating container', response, 'Failure creating container'), }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) build_env = DockerBuildEnvironment( version=self.version, @@ -572,15 +590,17 @@ def test_api_failure_on_docker_memory_limit(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': -1, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -680,6 +700,9 @@ def test_command_execution(self): 'exec_start.return_value': b'This is the return', 'exec_inspect.return_value': {'ExitCode': 1}, }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) build_env = DockerBuildEnvironment( version=self.version, @@ -700,15 +723,17 @@ def test_command_execution(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 1, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -775,6 +800,9 @@ def test_record_command_as_success(self): 'exec_start.return_value': b'This is the return', 'exec_inspect.return_value': {'ExitCode': 1}, }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) build_env = DockerBuildEnvironment( version=self.version, @@ -795,15 +823,17 @@ def test_record_command_as_success(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 0, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -835,6 +865,9 @@ def test_command_execution_cleanup_exception(self): 'Failure killing container', ) }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) build_env = DockerBuildEnvironment( version=self.version, @@ -850,15 +883,17 @@ def test_command_execution_cleanup_exception(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 0, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ @@ -939,6 +974,9 @@ def test_container_timeout(self): 'exec_start.return_value': b'This is the return', 'exec_inspect.return_value': {'ExitCode': 0}, }) + self.mocks.configure_mock( + 'api_v2.command', {'post.return_value': {'id': 1}} + ) build_env = DockerBuildEnvironment( version=self.version, @@ -954,15 +992,17 @@ def test_container_timeout(self): # api() is not called anymore, we use api_v2 instead self.assertFalse(self.mocks.api()(DUMMY_BUILD_ID).put.called) - # The command was saved + # The command was saved and updated command = build_env.commands[0] self.mocks.mocks['api_v2.command'].post.assert_called_once_with({ 'build': DUMMY_BUILD_ID, 'command': command.get_command(), 'description': command.description, + 'start_time': mock.ANY, + }) + self.mocks.mocks['api_v2.command']().patch.assert_called_once_with({ 'output': command.output, 'exit_code': 0, - 'start_time': command.start_time, 'end_time': command.end_time, }) self.mocks.mocks['api_v2.build']().put.assert_called_with({ diff --git a/readthedocs/templates/builds/build_detail.html b/readthedocs/templates/builds/build_detail.html index 4aa528eaba0..d94b92171a2 100644 --- a/readthedocs/templates/builds/build_detail.html +++ b/readthedocs/templates/builds/build_detail.html @@ -190,11 +190,13 @@

{% trans "Error" %}

-
+
-
+
{% trans "Command time:" %} 0s