Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove files after build #5680

Merged
merged 31 commits into from
Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
061066a
Fix tests
stsewd May 8, 2019
11debc3
Add task to remove files after build
stsewd May 8, 2019
0a6445e
Fix more tests
stsewd May 8, 2019
e44120b
Merge branch 'master' into remove-files-after-build
stsewd May 9, 2019
96ef713
Fix docstring
stsewd May 9, 2019
b3c8d63
Fix typos
stsewd May 9, 2019
9bde8d3
Only remove files from the current builder
stsewd May 13, 2019
726c7ea
Merge branch 'master' into remove-files-after-build
stsewd May 13, 2019
ac79017
Merge branch 'master' into remove-files-after-build
stsewd May 14, 2019
47f2b67
Update commands
stsewd May 14, 2019
c425869
Clean files after sync versions via webhooks
stsewd May 14, 2019
3f6095d
Add feature flag and lock
stsewd May 14, 2019
2bf07e5
Remove comment
stsewd May 14, 2019
b613bbb
Remove artifacts too
stsewd May 14, 2019
955286a
Merge branch 'master' into remove-files-after-build
stsewd May 17, 2019
9ba356c
Move lock to top level tasks
stsewd May 17, 2019
f806708
Merge branch 'master' into remove-files-after-build
stsewd May 30, 2019
ce44698
Put log.info
stsewd May 30, 2019
3245cd0
Merge branch 'master' into remove-files-after-build
stsewd Jun 3, 2019
cd350c2
Use try/finally blocks to run clean_build_files in the same server
stsewd Jun 4, 2019
8558c20
Revert lock
stsewd Jun 4, 2019
caeb3ab
Untouch files
stsewd Jun 4, 2019
faedd02
Merge branch 'master' into remove-files-after-build
stsewd Jun 5, 2019
eca17fe
Merge branch 'master' into remove-files-after-build
stsewd Jun 5, 2019
a65dc38
Refactor
stsewd Jun 6, 2019
2d6352d
Merge branch 'master' into remove-files-after-build
stsewd Jun 6, 2019
167dcbf
Fix tests
stsewd Jun 6, 2019
478935d
Fix mock
stsewd Jun 6, 2019
9ca1107
Add tests
stsewd Jun 6, 2019
3e3a532
Ignore artifacts path
stsewd Jun 6, 2019
2d045fa
Typo
stsewd Jun 7, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -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__)


Expand Down
7 changes: 6 additions & 1 deletion readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')),
Expand Down Expand Up @@ -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'),
),
)

Expand Down
44 changes: 39 additions & 5 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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.
Expand Down
56 changes: 56 additions & 0 deletions readthedocs/rtd_tests/tests/test_celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
100 changes: 42 additions & 58 deletions readthedocs/rtd_tests/tests/test_core_utils.py
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -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):

Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand All @@ -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."""
Expand Down
7 changes: 2 additions & 5 deletions readthedocs/rtd_tests/tests/test_privacy.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
# -*- coding: utf-8 -*-
import json
import logging

import mock
Expand All @@ -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):
Expand All @@ -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',
Expand Down