Merge pull request #5680 from stsewd/remove-files-after-build

Remove files after build

Author: stsewd

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__)
 
 

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'),
         ),
     )
 

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.

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):

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."""

readthedocs/rtd_tests/tests/test_privacy.py

@@ -1,5 +1,3 @@
-# -*- coding: utf-8 -*-
-import json
 import logging
 
 import mock
@@ -9,14 +7,15 @@
 
 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
 
 
 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',