diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 1146647a517..dbb8245c6fb 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -1,21 +1,20 @@ """Common utilty functions.""" -from __future__ import absolute_import - import errno import logging import os import re +from celery import chord, group from django.conf import settings from django.utils.functional import keep_lazy from django.utils.safestring import SafeText, mark_safe from django.utils.text import slugify as slugify_base -from celery import group, chord from readthedocs.builds.constants import BUILD_STATE_TRIGGERED from readthedocs.doc_builder.constants import DOCKER_LIMITS + log = logging.getLogger(__name__) diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 473638239d7..19f58f7fab0 100644 --- a/readthedocs/projects/models.py +++ b/readthedocs/projects/models.py @@ -1389,6 +1389,7 @@ def add_features(sender, **kwargs): USE_TESTING_BUILD_IMAGE = 'use_testing_build_image' SHARE_SPHINX_DOCTREE = 'share_sphinx_doctree' DEFAULT_TO_MKDOCS_0_17_3 = 'default_to_mkdocs_0_17_3' + CLEAN_AFTER_BUILD = 'clean_after_build' FEATURES = ( (USE_SPHINX_LATEST, _('Use latest version of Sphinx')), @@ -1423,7 +1424,11 @@ def add_features(sender, **kwargs): ), ( DEFAULT_TO_MKDOCS_0_17_3, - _('Install mkdocs 0.17.3 by default') + _('Install mkdocs 0.17.3 by default'), + ), + ( + CLEAN_AFTER_BUILD, + _('Clean all files used in the build process'), ), ) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 5f1360d0954..39fc4407043 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -60,7 +60,7 @@ ) from readthedocs.doc_builder.loader import get_builder_class from readthedocs.doc_builder.python_environments import Conda, Virtualenv -from readthedocs.projects.models import APIProject +from readthedocs.projects.models import APIProject, Feature from readthedocs.sphinx_domains.models import SphinxDomain from readthedocs.vcs_support import utils as vcs_support_utils from readthedocs.worker import app @@ -203,8 +203,11 @@ def validate_duplicate_reserved_versions(self, data): @app.task(max_retries=5, default_retry_delay=7 * 60) def sync_repository_task(version_pk): """Celery task to trigger VCS version sync.""" - step = SyncRepositoryTaskStep() - return step.run(version_pk) + try: + step = SyncRepositoryTaskStep() + return step.run(version_pk) + finally: + clean_build(version_pk) class SyncRepositoryTaskStep(SyncRepositoryMixin): @@ -280,8 +283,11 @@ def run(self, version_pk): # pylint: disable=arguments-differ ), ) def update_docs_task(self, version_pk, *args, **kwargs): - step = UpdateDocsTaskStep(task=self) - return step.run(version_pk, *args, **kwargs) + try: + step = UpdateDocsTaskStep(task=self) + return step.run(version_pk, *args, **kwargs) + finally: + clean_build(version_pk) class UpdateDocsTaskStep(SyncRepositoryMixin): @@ -1379,6 +1385,34 @@ def warn(self, msg): delete_queryset.delete() +def clean_build(version_pk): + """Clean the files used in the build of the given version.""" + version = Version.objects.get_object_or_log(pk=version_pk) + if ( + not version or + not version.project.has_feature(Feature.CLEAN_AFTER_BUILD) + ): + log.info( + 'Skipping build files deletetion for version: %s', + version_pk, + ) + return False + # NOTE: we are skipping the deletion of the `artifacts` dir + # because we are syncing the servers with an async task. + del_dirs = [ + os.path.join(version.project.doc_path, dir_, version.slug) + for dir_ in ('checkouts', 'envs', 'conda') + ] + try: + with version.project.repo_nonblockinglock(version): + log.info('Removing: %s', del_dirs) + remove_dirs(del_dirs) + except vcs_support_utils.LockTimeout: + log.info('Another task is running. Not removing: %s', del_dirs) + else: + return True + + def _manage_imported_files(version, path, commit): """ Update imported files for version. diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index a7f122a7d1e..543519bfebe 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -159,12 +159,68 @@ def test_no_notification_on_version_locked_error(self, mock_setup_vcs, mock_send mock_send_notifications.assert_not_called() self.assertTrue(result.successful()) + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.setup_python_environment', new=MagicMock) + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.setup_vcs', new=MagicMock) + @patch('readthedocs.doc_builder.environments.BuildEnvironment.update_build', new=MagicMock) + @patch('readthedocs.projects.tasks.clean_build') + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.build_docs') + def test_clean_build_after_update_docs(self, build_docs, clean_build): + version = self.project.versions.first() + build = get( + Build, project=self.project, + version=version, + ) + with mock_api(self.repo) as mapi: + result = tasks.update_docs_task.delay( + version.pk, + build_pk=build.pk, + record=False, + intersphinx=False, + ) + self.assertTrue(result.successful()) + clean_build.assert_called_with(version.pk) + + @patch('readthedocs.projects.tasks.clean_build') + @patch('readthedocs.projects.tasks.UpdateDocsTaskStep.run_setup') + def test_clean_build_after_failure_in_update_docs(self, run_setup, clean_build): + run_setup.side_effect = Exception() + version = self.project.versions.first() + build = get( + Build, project=self.project, + version=version, + ) + with mock_api(self.repo): + result = tasks.update_docs_task.delay( + version.pk, + build_pk=build.pk, + record=False, + intersphinx=False, + ) + clean_build.assert_called_with(version.pk) + def test_sync_repository(self): version = self.project.versions.get(slug=LATEST) with mock_api(self.repo): result = tasks.sync_repository_task.delay(version.pk) self.assertTrue(result.successful()) + @patch('readthedocs.projects.tasks.clean_build') + def test_clean_build_after_sync_repository(self, clean_build): + version = self.project.versions.get(slug=LATEST) + with mock_api(self.repo): + result = tasks.sync_repository_task.delay(version.pk) + self.assertTrue(result.successful()) + clean_build.assert_called_with(version.pk) + + @patch('readthedocs.projects.tasks.SyncRepositoryTaskStep.run') + @patch('readthedocs.projects.tasks.clean_build') + def test_clean_build_after_failure_in_sync_repository(self, clean_build, run_syn_repository): + run_syn_repository.side_effect = Exception() + version = self.project.versions.get(slug=LATEST) + with mock_api(self.repo): + result = tasks.sync_repository_task.delay(version.pk) + clean_build.assert_called_with(version.pk) + @patch('readthedocs.projects.tasks.api_v2') @patch('readthedocs.projects.models.Project.checkout_path') def test_check_duplicate_reserved_version_latest(self, checkout_path, api_v2): diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index c5cf86bd6d9..26205fbd160 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -1,19 +1,19 @@ """Test core util functions.""" import os -import mock -from mock import call +import mock from django.http import Http404 from django.test import TestCase from django_dynamic_fixture import get +from mock import call +from readthedocs.builds.constants import LATEST from readthedocs.builds.models import Version -from readthedocs.core.utils.general import wipe_version_via_slugs -from readthedocs.projects.tasks import remove_dirs from readthedocs.core.utils import slugify, trigger_build +from readthedocs.core.utils.general import wipe_version_via_slugs from readthedocs.projects.models import Project -from readthedocs.builds.constants import LATEST +from readthedocs.projects.tasks import remove_dirs class CoreUtilTests(TestCase): @@ -54,15 +54,13 @@ def test_trigger_build_when_version_not_provided_default_version_exist(self, upd 'build_pk': mock.ANY, } - update_docs_task.signature.assert_has_calls([ - mock.call( - args=(version_1.pk,), - kwargs=kwargs, - options=mock.ANY, - immutable=True, - ), - ]) - + update_docs_task.signature.assert_called_with( + args=(version_1.pk,), + kwargs=kwargs, + options=mock.ANY, + immutable=True, + ) + @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_when_version_not_provided_default_version_doesnt_exist(self, update_docs_task): @@ -78,14 +76,12 @@ def test_trigger_build_when_version_not_provided_default_version_doesnt_exist(se 'build_pk': mock.ANY, } - update_docs_task.signature.assert_has_calls([ - mock.call( - args=(version.pk,), - kwargs=kwargs, - options=mock.ANY, - immutable=True, - ), - ]) + update_docs_task.signature.assert_called_with( + args=(version.pk,), + kwargs=kwargs, + options=mock.ANY, + immutable=True, + ) @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_custom_queue(self, update_docs): @@ -102,15 +98,12 @@ def test_trigger_custom_queue(self, update_docs): 'time_limit': 720, 'soft_time_limit': 600, } - update_docs.signature.assert_has_calls([ - mock.call( - args=(self.version.pk,), - kwargs=kwargs, - options=options, - immutable=True, - ), - ]) - update_docs.signature().apply_async.assert_called() + update_docs.signature.assert_called_with( + args=(self.version.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ) @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_time_limit(self, update_docs): @@ -126,15 +119,12 @@ def test_trigger_build_time_limit(self, update_docs): 'time_limit': 720, 'soft_time_limit': 600, } - update_docs.signature.assert_has_calls([ - mock.call( - args=(self.version.pk,), - kwargs=kwargs, - options=options, - immutable=True, - ), - ]) - update_docs.signature().apply_async.assert_called() + update_docs.signature.assert_called_with( + args=(self.version.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ) @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_invalid_time_limit(self, update_docs): @@ -151,15 +141,12 @@ def test_trigger_build_invalid_time_limit(self, update_docs): 'time_limit': 720, 'soft_time_limit': 600, } - update_docs.signature.assert_has_calls([ - mock.call( - args=(self.version.pk,), - kwargs=kwargs, - options=options, - immutable=True, - ), - ]) - update_docs.signature().apply_async.assert_called() + update_docs.signature.assert_called_with( + args=(self.version.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ) @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_rounded_time_limit(self, update_docs): @@ -176,15 +163,12 @@ def test_trigger_build_rounded_time_limit(self, update_docs): 'time_limit': 3, 'soft_time_limit': 3, } - update_docs.signature.assert_has_calls([ - mock.call( - args=(self.version.pk,), - kwargs=kwargs, - options=options, - immutable=True, - ), - ]) - update_docs.signature().apply_async.assert_called() + update_docs.signature.assert_called_with( + args=(self.version.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ) def test_slugify(self): """Test additional slugify.""" diff --git a/readthedocs/rtd_tests/tests/test_privacy.py b/readthedocs/rtd_tests/tests/test_privacy.py index bac259253ae..7c75f14ff10 100644 --- a/readthedocs/rtd_tests/tests/test_privacy.py +++ b/readthedocs/rtd_tests/tests/test_privacy.py @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- -import json import logging import mock @@ -9,7 +7,6 @@ from readthedocs.builds.constants import LATEST from readthedocs.builds.models import Build, Version -from readthedocs.projects import tasks from readthedocs.projects.forms import UpdateProjectForm from readthedocs.projects.models import Project @@ -17,6 +14,8 @@ log = logging.getLogger(__name__) +@mock.patch('readthedocs.projects.tasks.clean_build', new=mock.MagicMock) +@mock.patch('readthedocs.projects.tasks.update_docs_task.signature', new=mock.MagicMock) class PrivacyTests(TestCase): def setUp(self): @@ -28,8 +27,6 @@ def setUp(self): self.tester.set_password('test') self.tester.save() - tasks.update_docs_task.delay = mock.Mock() - def _create_kong( self, privacy_level='private', version_privacy_level='private',