From c0a951a8333fdc06bb32b78222c5eed8ef5d5b82 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 26 Jan 2022 21:27:40 +0100 Subject: [PATCH 01/10] 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 | 7 ++++++ 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(+), 1 deletion(-) 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 59b877fbb41..2d81fbc973d 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -682,6 +682,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 e3fe6e52237..df928d89498 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -229,7 +229,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 d09524884ba..6a033b70bf6 100644 --- a/readthedocs/projects/tasks/builds.py +++ b/readthedocs/projects/tasks/builds.py @@ -52,6 +52,7 @@ BuildUserError, BuildMaxConcurrencyError, DuplicatedBuildError, + BuildCancelled, ProjectBuildsSkippedError, YAMLParseError, MkDocsYAMLParseError, @@ -240,6 +241,7 @@ class UpdateDocsTask(SyncRepositoryMixin, Task): ProjectBuildsSkippedError, ConfigError, YAMLParseError, + BuildCancelled, BuildUserError, RepositoryError, MkDocsYAMLParseError, @@ -265,10 +267,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.data.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 %} + +
+