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 6 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
23 changes: 14 additions & 9 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,21 @@

"""Common utilty functions."""

from __future__ import absolute_import

import errno
import logging
import os
import re

from celery import chain, 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 Expand Up @@ -81,7 +80,7 @@ def prepare_build(
# Avoid circular import
from readthedocs.builds.models import Build
from readthedocs.projects.models import Project
from readthedocs.projects.tasks import update_docs_task
from readthedocs.projects.tasks import clean_build_task, update_docs_task

build = None

Expand Down Expand Up @@ -130,11 +129,17 @@ def prepare_build(
options['time_limit'] = int(time_limit * 1.2)

return (
update_docs_task.signature(
args=(project.pk,),
kwargs=kwargs,
options=options,
immutable=True,
chain(
update_docs_task.signature(
args=(project.pk,),
kwargs=kwargs,
options=options,
immutable=True,
),
clean_build_task.signature(
humitos marked this conversation as resolved.
Show resolved Hide resolved
args=(version.pk,),
immutable=True,
)
),
build,
)
Expand Down
14 changes: 14 additions & 0 deletions readthedocs/projects/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -1315,6 +1315,20 @@ def warn(self, msg):
delete_queryset.delete()


@app.task(max_retries=5, default_retry_delay=7 * 60)
def clean_build_task(version_id):
stsewd marked this conversation as resolved.
Show resolved Hide resolved
"""Clean the files used in the build of the given version."""
version = Version.objects.get_object_or_log(pk=version_id)
if not version:
return
stsewd marked this conversation as resolved.
Show resolved Hide resolved
del_dirs = [
os.path.join(version.project.doc_path, dir_, version.slug)
for dir_ in ('checkouts', 'envs', 'conda')
]
for del_dir in del_dirs:
broadcast(type='build', task=remove_dirs, args=[(del_dir,)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely shouldn't be a broadcast. We only do locking per-server, so doing this will definitely break things.



def _manage_imported_files(version, path, commit):
"""
Update imported files for version.
Expand Down
79 changes: 61 additions & 18 deletions readthedocs/rtd_tests/tests/test_core_utils.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
# -*- coding: utf-8 -*-
"""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 @@ -64,7 +63,7 @@ def test_trigger_build_when_version_not_provided_default_version_exist(self, upd
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 @@ -90,8 +89,10 @@ def test_trigger_build_when_version_not_provided_default_version_doesnt_exist(se
),
])

@mock.patch('readthedocs.projects.tasks.clean_build_task')
@mock.patch('readthedocs.projects.tasks.update_docs_task')
def test_trigger_custom_queue(self, update_docs):
@mock.patch('readthedocs.core.utils.chain')
def test_trigger_custom_queue(self, chain, update_docs, clean_build_task):
"""Use a custom queue when routing the task."""
self.project.build_queue = 'build03'
trigger_build(project=self.project, version=self.version)
Expand All @@ -106,18 +107,23 @@ def test_trigger_custom_queue(self, update_docs):
'time_limit': 720,
'soft_time_limit': 600,
}
update_docs.signature.assert_has_calls([
mock.call(
chain.assert_called_with(
update_docs.signature(
args=(self.project.pk,),
kwargs=kwargs,
options=options,
immutable=True,
),
])
update_docs.signature().apply_async.assert_called()
clean_build_task.signature(
args=(self.version.pk,),
immutable=True,
),
)

@mock.patch('readthedocs.projects.tasks.clean_build_task')
@mock.patch('readthedocs.projects.tasks.update_docs_task')
def test_trigger_build_time_limit(self, update_docs):
@mock.patch('readthedocs.core.utils.chain')
def test_trigger_build_time_limit(self, chain, update_docs, clean_build_task):
"""Pass of time limit."""
trigger_build(project=self.project, version=self.version)
kwargs = {
Expand All @@ -139,10 +145,23 @@ def test_trigger_build_time_limit(self, update_docs):
immutable=True,
),
])
update_docs.signature().apply_async.assert_called()
chain.assert_called_with(
update_docs.signature(
args=(self.project.pk,),
kwargs=kwargs,
options=options,
immutable=True,
),
clean_build_task.signature(
args=(self.version.pk,),
immutable=True,
),
)

@mock.patch('readthedocs.projects.tasks.clean_build_task')
@mock.patch('readthedocs.projects.tasks.update_docs_task')
def test_trigger_build_invalid_time_limit(self, update_docs):
@mock.patch('readthedocs.core.utils.chain')
def test_trigger_build_invalid_time_limit(self, chain, update_docs, clean_build_task):
"""Time limit as string."""
self.project.container_time_limit = '200s'
trigger_build(project=self.project, version=self.version)
Expand All @@ -165,10 +184,23 @@ def test_trigger_build_invalid_time_limit(self, update_docs):
immutable=True,
),
])
update_docs.signature().apply_async.assert_called()
chain.assert_called_with(
update_docs.signature(
args=(self.project.pk,),
kwargs=kwargs,
options=options,
immutable=True,
),
clean_build_task.signature(
args=(self.version.pk,),
immutable=True,
),
)

@mock.patch('readthedocs.projects.tasks.clean_build_task')
@mock.patch('readthedocs.projects.tasks.update_docs_task')
def test_trigger_build_rounded_time_limit(self, update_docs):
@mock.patch('readthedocs.core.utils.chain')
def test_trigger_build_rounded_time_limit(self, chain, update_docs, clean_build_task):
"""Time limit should round down."""
self.project.container_time_limit = 3
trigger_build(project=self.project, version=self.version)
Expand All @@ -191,7 +223,18 @@ def test_trigger_build_rounded_time_limit(self, update_docs):
immutable=True,
),
])
update_docs.signature().apply_async.assert_called()
chain.assert_called_with(
update_docs.signature(
args=(self.project.pk,),
kwargs=kwargs,
options=options,
immutable=True,
),
clean_build_task.signature(
args=(self.version.pk,),
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_task.signature', 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