From 60efd912409055f6a15077e8572addaa00c5b9e3 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 26 Jan 2022 21:27:40 +0100 Subject: [PATCH] Build: ability to cancel a running build from dashboard We tried to implement this in another opportunities (see https://github.com/readthedocs/readthedocs.org/pull/7031) but the build process was really complex and we had to manage the exception in multiple places. After implementing Celery Handlers, we can just raise the exception when attending to the proper signal coming from `app.control.revoke` and handle it properly from `on_failure` task's method. All the initial local tests were great! --- .../builds/migrations/0038_track_task_id.py | 18 ++++++++++++++ readthedocs/builds/models.py | 8 ++++++- readthedocs/builds/views.py | 24 +++++++++++++++++++ readthedocs/core/utils/__init__.py | 8 ++++++- readthedocs/doc_builder/exceptions.py | 4 ++++ readthedocs/projects/tasks/builds.py | 9 +++++++ .../templates/builds/build_detail.html | 5 ++++ 7 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 readthedocs/builds/migrations/0038_track_task_id.py diff --git a/readthedocs/builds/migrations/0038_track_task_id.py b/readthedocs/builds/migrations/0038_track_task_id.py new file mode 100644 index 00000000000..411b656dc89 --- /dev/null +++ b/readthedocs/builds/migrations/0038_track_task_id.py @@ -0,0 +1,18 @@ +# Generated by Django 3.2.11 on 2022-01-26 20:10 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('builds', '0037_alter_build_cold_storage'), + ] + + operations = [ + migrations.AddField( + model_name='build', + name='task_id', + field=models.CharField(blank=True, max_length=36, null=True, verbose_name='Celery task id'), + ), + ] diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index ed128313631..4eb0de4af74 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -5,7 +5,6 @@ import os.path import re from functools import partial -from shutil import rmtree import regex from django.conf import settings @@ -667,6 +666,13 @@ class Build(models.Model): help_text='Build steps stored outside the database.', ) + task_id = models.CharField( + _('Celery task id'), + max_length=36, + null=True, + blank=True, + ) + # Managers objects = BuildQuerySet.as_manager() # Only include BRANCH, TAG, UNKNOWN type Version builds. diff --git a/readthedocs/builds/views.py b/readthedocs/builds/views.py index 0042e8fe7d7..50a826ed89b 100644 --- a/readthedocs/builds/views.py +++ b/readthedocs/builds/views.py @@ -1,5 +1,7 @@ """Views for builds app.""" +import signal + import structlog import textwrap from urllib.parse import urlparse @@ -21,6 +23,12 @@ from readthedocs.doc_builder.exceptions import BuildAppError from readthedocs.projects.models import Project +try: + from readthedocsinc.worker import app +except ImportError: + from readthedocs.worker import app + + log = structlog.get_logger(__name__) @@ -148,6 +156,22 @@ class BuildDetail(BuildBase, DetailView): pk_url_kwarg = 'build_pk' + @method_decorator(login_required) + def post(self, request, project_slug, build_pk): + project = get_object_or_404(Project, slug=project_slug) + build = get_object_or_404(Build, pk=build_pk) + + if not AdminPermission.is_admin(request.user, project): + return HttpResponseForbidden() + + # NOTE: `terminate=True` is required for the child to attend our call + # immediately. Otherwise, it finishes the task. + app.control.revoke(build.task_id, signal=signal.SIGINT, terminate=True) + + return HttpResponseRedirect( + reverse('builds_detail', args=[project.slug, build.pk]), + ) + def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) context['project'] = self.project diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 68d4aee700b..95acd086fa4 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -239,7 +239,13 @@ def trigger_build(project, version=None, commit=None): # Build was skipped return (None, None) - return (update_docs_task.apply_async(), build) + task = update_docs_task.apply_async() + + # Store the task_id in the build object to be able to cancel it later. + build.task_id = task.id + build.save() + + return task, build def send_email( diff --git a/readthedocs/doc_builder/exceptions.py b/readthedocs/doc_builder/exceptions.py index aeca5c2145c..f1221266dfc 100644 --- a/readthedocs/doc_builder/exceptions.py +++ b/readthedocs/doc_builder/exceptions.py @@ -59,6 +59,10 @@ class DuplicatedBuildError(BuildUserError): status = BUILD_STATUS_DUPLICATED +class BuildCancelled(BuildUserError): + message = gettext_noop('Build cancelled by user.') + + class MkDocsYAMLParseError(BuildUserError): GENERIC_WITH_PARSE_EXCEPTION = gettext_noop( 'Problem parsing MkDocs YAML configuration. {exception}', diff --git a/readthedocs/projects/tasks/builds.py b/readthedocs/projects/tasks/builds.py index 303c09ac053..8512f1f6f69 100644 --- a/readthedocs/projects/tasks/builds.py +++ b/readthedocs/projects/tasks/builds.py @@ -51,6 +51,7 @@ BuildUserError, BuildMaxConcurrencyError, DuplicatedBuildError, + BuildCancelled, ProjectBuildsSkippedError, YAMLParseError, ) @@ -206,6 +207,7 @@ class UpdateDocsTask(SyncRepositoryMixin, Task): ProjectBuildsSkippedError, ConfigError, YAMLParseError, + BuildCancelled, ) acks_late = True @@ -221,10 +223,17 @@ def _setup_sigterm(self): def sigterm_received(*args, **kwargs): log.warning('SIGTERM received. Waiting for build to stop gracefully after it finishes.') + def sigint_received(*args, **kwargs): + log.warning('SIGINT received. Cancelling the build running.') + raise BuildCancelled + # Do not send the SIGTERM signal to children (pip is automatically killed when # receives SIGTERM and make the build to fail one command and stop build) signal.signal(signal.SIGTERM, sigterm_received) + + signal.signal(signal.SIGINT, sigint_received) + def _check_concurrency_limit(self): try: response = api_v2.build.concurrent.get(project__slug=self.project.slug) diff --git a/readthedocs/templates/builds/build_detail.html b/readthedocs/templates/builds/build_detail.html index 3aca7c45d0f..c376d2f0dcd 100644 --- a/readthedocs/templates/builds/build_detail.html +++ b/readthedocs/templates/builds/build_detail.html @@ -30,6 +30,11 @@ {% block content %}
+
+ {% csrf_token %} + +
+