diff --git a/readthedocs/builds/models.py b/readthedocs/builds/models.py index 78d0799507b..70ac5ed19da 100644 --- a/readthedocs/builds/models.py +++ b/readthedocs/builds/models.py @@ -215,7 +215,7 @@ def delete(self, *args, **kwargs): # pylint: disable=arguments-differ log.info('Removing files for version %s', self.slug) broadcast( type='app', - task=tasks.clear_artifacts, + task=tasks.remove_dirs, args=[self.get_artifact_paths()], ) project_pk = self.project.pk diff --git a/readthedocs/core/views/__init__.py b/readthedocs/core/views/__init__.py index 6f25e9bd92b..f3c67b56b64 100644 --- a/readthedocs/core/views/__init__.py +++ b/readthedocs/core/views/__init__.py @@ -19,7 +19,7 @@ from readthedocs.builds.models import Version from readthedocs.core.utils import broadcast from readthedocs.projects.models import Project, ImportedFile -from readthedocs.projects.tasks import remove_dir +from readthedocs.projects.tasks import remove_dirs from readthedocs.redirects.utils import get_redirect_response log = logging.getLogger(__name__) @@ -89,7 +89,7 @@ def wipe_version(request, project_slug, version_slug): os.path.join(version.project.doc_path, 'conda', version.slug), ] for del_dir in del_dirs: - broadcast(type='build', task=remove_dir, args=[del_dir]) + broadcast(type='build', task=remove_dirs, args=[(del_dir,)]) return redirect('project_version_list', project_slug) return render( request, diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index 4640a7abc73..9f6da41ddd9 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -31,11 +31,11 @@ WebHook, ) from .notifications import ( - ResourceUsageNotification, DeprecatedBuildWebhookNotification, DeprecatedGitHubWebhookNotification, + ResourceUsageNotification, ) -from .tasks import remove_dir +from .tasks import remove_dirs class ProjectSendNotificationView(SendNotificationView): @@ -182,7 +182,11 @@ def delete_selected_and_artifacts(self, request, queryset): """ if request.POST.get('post'): for project in queryset: - broadcast(type='app', task=remove_dir, args=[project.doc_path]) + broadcast( + type='app', + task=remove_dirs, + args=[(project.doc_path,)], + ) return delete_selected(self, request, queryset) def get_actions(self, request): diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index bcc81913753..904555a971a 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -880,19 +880,19 @@ def sync_files(project_pk, version_pk, hostname=None, html=False, # Clean up unused artifacts version = Version.objects.get(pk=version_pk) if not pdf: - remove_dir( + remove_dirs([ version.project.get_production_media_path( type_='pdf', version_slug=version.slug, ), - ) + ]) if not epub: - remove_dir( + remove_dirs([ version.project.get_production_media_path( type_='epub', version_slug=version.slug, ), - ) + ]) # Sync files to the web servers move_files( @@ -1327,27 +1327,18 @@ def update_static_metadata(project_pk, path=None): # Random Tasks @app.task() -def remove_dir(path): - """ - Remove a directory on the build/celery server. - - This is mainly a wrapper around shutil.rmtree so that app servers can kill - things on the build server. +def remove_dirs(paths): """ - log.info('Removing %s', path) - shutil.rmtree(path, ignore_errors=True) + Remove artifacts from servers. + This is mainly a wrapper around shutil.rmtree so that we can remove things across + every instance of a type of server (eg. all builds or all webs). -@app.task() -def clear_artifacts(paths): - """ - Remove artifacts from the web servers. - - :param paths: list containing PATHs where production media is on disk - (usually ``Version.get_artifact_paths``) + :param paths: list containing PATHs where file is on disk """ for path in paths: - remove_dir(path) + log.info('Removing %s', path) + shutil.rmtree(path, ignore_errors=True) @app.task(queue='web') diff --git a/readthedocs/projects/views/private.py b/readthedocs/projects/views/private.py index 330c22b5b7d..b5d2fbe1056 100644 --- a/readthedocs/projects/views/private.py +++ b/readthedocs/projects/views/private.py @@ -192,7 +192,7 @@ def project_version_detail(request, project_slug, version_slug): log.info('Removing files for version %s', version.slug) broadcast( type='app', - task=tasks.clear_artifacts, + task=tasks.remove_dirs, args=[version.get_artifact_paths()], ) version.built = False @@ -221,7 +221,11 @@ def project_delete(request, project_slug): ) if request.method == 'POST': - broadcast(type='app', task=tasks.remove_dir, args=[project.doc_path]) + broadcast( + type='app', + task=tasks.remove_dirs, + args=[(project.doc_path,)] + ) project.delete() messages.success(request, _('Project deleted')) project_dashboard = reverse('projects_dashboard') @@ -704,7 +708,7 @@ def project_version_delete_html(request, project_slug, version_slug): version.save() broadcast( type='app', - task=tasks.clear_artifacts, + task=tasks.remove_dirs, args=[version.get_artifact_paths()], ) else: diff --git a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py index 54111314cee..6898c5bf136 100644 --- a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py +++ b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py @@ -60,7 +60,7 @@ def test_project_ban_multiple_owners(self): @mock.patch('readthedocs.projects.admin.broadcast') def test_project_delete(self, broadcast): """Test project and artifacts are removed""" - from readthedocs.projects.tasks import remove_dir + from readthedocs.projects.tasks import remove_dirs action_data = { ACTION_CHECKBOX_NAME: [self.project.pk], 'action': 'delete_selected', @@ -74,6 +74,6 @@ def test_project_delete(self, broadcast): self.assertFalse(Project.objects.filter(pk=self.project.pk).exists()) broadcast.assert_has_calls([ mock.call( - type='app', task=remove_dir, args=[self.project.doc_path] + type='app', task=remove_dirs, args=[(self.project.doc_path,)] ), ]) diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index 3bc4fd6062f..7de03a97bf2 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -47,10 +47,10 @@ def tearDown(self): shutil.rmtree(self.repo) super(TestCeleryBuilding, self).tearDown() - def test_remove_dir(self): + def test_remove_dirs(self): directory = mkdtemp() self.assertTrue(exists(directory)) - result = tasks.remove_dir.delay(directory) + result = tasks.remove_dirs.delay((directory,)) self.assertTrue(result.successful()) self.assertFalse(exists(directory)) @@ -59,14 +59,14 @@ def test_clear_artifacts(self): directory = self.project.get_production_media_path(type_='pdf', version_slug=version.slug) os.makedirs(directory) self.assertTrue(exists(directory)) - result = tasks.clear_artifacts.delay(paths=version.get_artifact_paths()) + result = tasks.remove_dirs.delay(paths=version.get_artifact_paths()) self.assertTrue(result.successful()) self.assertFalse(exists(directory)) directory = version.project.rtd_build_path(version=version.slug) os.makedirs(directory) self.assertTrue(exists(directory)) - result = tasks.clear_artifacts.delay(paths=version.get_artifact_paths()) + result = tasks.remove_dirs.delay(paths=version.get_artifact_paths()) self.assertTrue(result.successful()) self.assertFalse(exists(directory)) diff --git a/readthedocs/rtd_tests/tests/test_project_views.py b/readthedocs/rtd_tests/tests/test_project_views.py index c6e4a93f884..cf437f29d62 100644 --- a/readthedocs/rtd_tests/tests/test_project_views.py +++ b/readthedocs/rtd_tests/tests/test_project_views.py @@ -379,8 +379,8 @@ def test_delete_project(self): self.assertFalse(Project.objects.filter(slug='pip').exists()) broadcast.assert_called_with( type='app', - task=tasks.remove_dir, - args=[project.doc_path]) + task=tasks.remove_dirs, + args=[(project.doc_path,)]) def test_subproject_create(self): project = get(Project, slug='pip', users=[self.user])