From 302cea6db0613b7cfd77e0c34d8838385fc75559 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 17 Aug 2017 09:19:07 -0700 Subject: [PATCH 01/11] Use new Celery, use new application pattern * Use modern celery * Drop djcelery * New pattern for starting celery * Bump redis to 2.10.6 to avoid startup bug, change autodiscover call --- readthedocs/__init__.py | 1 + .../core/management/commands/update_api.py | 3 +- .../core/management/commands/update_repos.py | 25 +++++++----- .../management/commands/update_versions.py | 3 +- readthedocs/core/tasks.py | 4 +- readthedocs/core/utils/__init__.py | 3 +- readthedocs/core/utils/tasks/retrieve.py | 5 ++- readthedocs/oauth/tasks.py | 3 +- readthedocs/projects/tasks.py | 39 ++++++++++--------- readthedocs/rtd_tests/tests/test_celery.py | 9 +++-- .../rtd_tests/tests/test_core_utils.py | 18 ++++----- readthedocs/settings/base.py | 7 ---- readthedocs/worker.py | 19 +++++++++ requirements/pip.txt | 5 +-- 14 files changed, 85 insertions(+), 59 deletions(-) create mode 100644 readthedocs/worker.py diff --git a/readthedocs/__init__.py b/readthedocs/__init__.py index e69de29bb2d..9f175088761 100644 --- a/readthedocs/__init__.py +++ b/readthedocs/__init__.py @@ -0,0 +1 @@ +from .tasks import app diff --git a/readthedocs/core/management/commands/update_api.py b/readthedocs/core/management/commands/update_api.py index 27d176f5ebb..e8433513bdd 100644 --- a/readthedocs/core/management/commands/update_api.py +++ b/readthedocs/core/management/commands/update_api.py @@ -33,4 +33,5 @@ def handle(self, *args, **options): project_data = api.project(slug).get() p = APIProject(**project_data) log.info("Building %s", p) - tasks.update_docs.run(pk=p.pk, docker=docker) + update_docs = tasks.UpdateDocsTask() + update_docs.run(pk=p.pk, docker=docker) diff --git a/readthedocs/core/management/commands/update_repos.py b/readthedocs/core/management/commands/update_repos.py index 142ad19e5e2..536e4c018e2 100644 --- a/readthedocs/core/management/commands/update_repos.py +++ b/readthedocs/core/management/commands/update_repos.py @@ -54,9 +54,11 @@ def handle(self, *args, **options): for version in Version.objects.filter(project__slug=slug, active=True, uploaded=False): - tasks.update_docs.run(pk=version.project_id, - record=False, - version_pk=version.pk) + tasks.UpdateDocsTask().run( + pk=version.project_id, + record=False, + version_pk=version.pk + ) else: p = Project.all_objects.get(slug=slug) log.info("Building %s", p) @@ -66,12 +68,17 @@ def handle(self, *args, **options): log.info("Updating all versions") for version in Version.objects.filter(active=True, uploaded=False): - tasks.update_docs.run(pk=version.project_id, - record=record, - force=force, - version_pk=version.pk) + tasks.UpdateDocsTask().run( + pk=version.project_id, + record=record, + force=force, + version_pk=version.pk + ) else: log.info("Updating all docs") for project in Project.objects.all(): - tasks.update_docs.run(pk=project.pk, record=record, - force=force) + tasks.UpdateDocsTask().run( + pk=project.pk, + record=record, + force=force + ) diff --git a/readthedocs/core/management/commands/update_versions.py b/readthedocs/core/management/commands/update_versions.py index 88e8d7c410c..c3ae6fa8e1b 100644 --- a/readthedocs/core/management/commands/update_versions.py +++ b/readthedocs/core/management/commands/update_versions.py @@ -4,7 +4,7 @@ from django.core.management.base import BaseCommand from readthedocs.builds.models import Version -from readthedocs.projects.tasks import update_docs +from readthedocs.projects.tasks import UpdateDocsTask class Command(BaseCommand): @@ -13,5 +13,6 @@ class Command(BaseCommand): def handle(self, *args, **options): for version in Version.objects.filter(active=True, built=False): + update_docs = UpdateDocsTask() update_docs.run(version.project_id, record=False, version_pk=version.pk) diff --git a/readthedocs/core/tasks.py b/readthedocs/core/tasks.py index 7c57cd90a4f..b3efbab305c 100644 --- a/readthedocs/core/tasks.py +++ b/readthedocs/core/tasks.py @@ -3,7 +3,7 @@ from __future__ import absolute_import import logging -from celery import task +from celery import shared_task from django.conf import settings from django.core.mail import EmailMultiAlternatives from django.template.loader import get_template @@ -15,7 +15,7 @@ EMAIL_TIME_LIMIT = 30 -@task(queue='web', time_limit=EMAIL_TIME_LIMIT) +@shared_task(queue='web', time_limit=EMAIL_TIME_LIMIT) def send_email_task(recipient, subject, template, template_html, context=None): """Send multipart email diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index fca8e13f96f..f42b31bb6a4 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -77,7 +77,7 @@ def trigger_build(project, version=None, record=True, force=False, basic=False): will be prefixed with ``build-`` to unify build queue names. """ # Avoid circular import - from readthedocs.projects.tasks import update_docs + from readthedocs.projects.tasks import UpdateDocsTask from readthedocs.builds.models import Build if project.skip: @@ -121,6 +121,7 @@ def trigger_build(project, version=None, record=True, force=False, basic=False): options['soft_time_limit'] = time_limit options['time_limit'] = int(time_limit * 1.2) + update_docs = UpdateDocsTask() update_docs.apply_async(kwargs=kwargs, **options) return build diff --git a/readthedocs/core/utils/tasks/retrieve.py b/readthedocs/core/utils/tasks/retrieve.py index f33afab8094..9da3c581601 100644 --- a/readthedocs/core/utils/tasks/retrieve.py +++ b/readthedocs/core/utils/tasks/retrieve.py @@ -1,7 +1,6 @@ """Utilities for retrieving task data.""" from __future__ import absolute_import -from djcelery import celery as celery_app from celery.result import AsyncResult @@ -20,6 +19,8 @@ def get_task_data(task_id): meta data has no ``'task_name'`` key set. """ + from readthedocs.worker import app + result = AsyncResult(task_id) state, info = result.state, result.info if state == 'PENDING': @@ -27,7 +28,7 @@ def get_task_data(task_id): if 'task_name' not in info: raise TaskNotFound(task_id) try: - task = celery_app.tasks[info['task_name']] + task = app.tasks[info['task_name']] except KeyError: raise TaskNotFound(task_id) return task, state, info diff --git a/readthedocs/oauth/tasks.py b/readthedocs/oauth/tasks.py index b345d443046..7f95f9610ea 100644 --- a/readthedocs/oauth/tasks.py +++ b/readthedocs/oauth/tasks.py @@ -2,7 +2,6 @@ from __future__ import absolute_import from django.contrib.auth.models import User -from djcelery import celery as celery_app from readthedocs.core.utils.tasks import PublicTask from readthedocs.core.utils.tasks import permission_check @@ -22,4 +21,4 @@ def run_public(self, user_id): service.sync() -sync_remote_repositories = celery_app.tasks[SyncRemoteRepositories.name] +sync_remote_repositories = SyncRemoteRepositories() diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index a52fa80c3af..3ca5b776ac0 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -16,7 +16,7 @@ import requests from builtins import str -from celery import task, Task +from celery import shared_task, Task from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings from django.core.urlresolvers import reverse @@ -53,6 +53,7 @@ from readthedocs.search.parse_json import process_all_json_files from readthedocs.search.utils import process_mkdocs_json from readthedocs.vcs_support import utils as vcs_support_utils +from readthedocs.worker import app log = logging.getLogger(__name__) @@ -80,7 +81,7 @@ class UpdateDocsTask(Task): max_retries = 5 default_retry_delay = (7 * 60) - name = 'update_docs' + name = __name__ + '.update_docs' def __init__(self, build_env=None, python_env=None, config=None, force=False, search=True, localmedia=True, @@ -490,10 +491,10 @@ def send_notifications(self): send_notifications.delay(self.version.pk, build_pk=self.build['id']) -update_docs = celery_app.tasks[UpdateDocsTask.name] +app.register_task(UpdateDocsTask()) -@task() +@shared_task() def update_imported_docs(version_pk): """ Check out or update the given project's repository @@ -572,7 +573,7 @@ def update_imported_docs(version_pk): # Web tasks -@task(queue='web') +@shared_task(queue='web') def sync_files(project_pk, version_pk, hostname=None, html=False, localmedia=False, search=False, pdf=False, epub=False): """Sync build artifacts to application instances @@ -604,7 +605,7 @@ def sync_files(project_pk, version_pk, hostname=None, html=False, update_static_metadata(project_pk) -@task(queue='web') +@shared_task(queue='web') def move_files(version_pk, hostname, html=False, localmedia=False, search=False, pdf=False, epub=False): """Task to move built documentation to web servers @@ -670,7 +671,7 @@ def move_files(version_pk, hostname, html=False, localmedia=False, search=False, Syncer.copy(from_path, to_path, host=hostname) -@task(queue='web') +@shared_task(queue='web') def update_search(version_pk, commit, delete_non_commit_files=True): """Task to update search indexes @@ -705,7 +706,7 @@ def update_search(version_pk, commit, delete_non_commit_files=True): ) -@task(queue='web') +@shared_task(queue='web') def symlink_project(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -713,7 +714,7 @@ def symlink_project(project_pk): sym.run() -@task(queue='web') +@shared_task(queue='web') def symlink_domain(project_pk, domain_pk, delete=False): project = Project.objects.get(pk=project_pk) domain = Domain.objects.get(pk=domain_pk) @@ -725,7 +726,7 @@ def symlink_domain(project_pk, domain_pk, delete=False): sym.symlink_cnames(domain) -@task(queue='web') +@shared_task(queue='web') def symlink_subproject(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -733,7 +734,7 @@ def symlink_subproject(project_pk): sym.symlink_subprojects() -@task(queue='web') +@shared_task(queue='web') def fileify(version_pk, commit): """ Create ImportedFile objects for all of a version's files. @@ -809,7 +810,7 @@ def _manage_imported_files(version, path, commit): purge(cdn_ids[version.project.slug], changed_files) -@task(queue='web') +@shared_task(queue='web') def send_notifications(version_pk, build_pk): version = Version.objects.get(pk=version_pk) build = Build.objects.get(pk=build_pk) @@ -878,7 +879,7 @@ def webhook_notification(version, build, hook_url): requests.post(hook_url, data=data) -@task(queue='web') +@shared_task(queue='web') def update_static_metadata(project_pk, path=None): """Update static metadata JSON file @@ -924,7 +925,7 @@ def update_static_metadata(project_pk, path=None): # Random Tasks -@task() +@shared_task() def remove_dir(path): """ Remove a directory on the build/celery server. @@ -936,7 +937,7 @@ def remove_dir(path): shutil.rmtree(path, ignore_errors=True) -@task() +@shared_task() def clear_artifacts(version_pk): """Remove artifacts from the web servers""" version = Version.objects.get(pk=version_pk) @@ -946,7 +947,7 @@ def clear_artifacts(version_pk): clear_html_artifacts(version) -@task() +@shared_task() def clear_pdf_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -954,7 +955,7 @@ def clear_pdf_artifacts(version): type_='pdf', version_slug=version.slug)) -@task() +@shared_task() def clear_epub_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -962,7 +963,7 @@ def clear_epub_artifacts(version): type_='epub', version_slug=version.slug)) -@task() +@shared_task() def clear_htmlzip_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -970,7 +971,7 @@ def clear_htmlzip_artifacts(version): type_='htmlzip', version_slug=version.slug)) -@task() +@shared_task() def clear_html_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index d73c2c58fca..b4ac4530152 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -72,7 +72,8 @@ def test_update_docs(self): build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - result = tasks.update_docs.delay( + update_docs = tasks.UpdateDocsTask() + result = update_docs.delay( self.project.pk, build_pk=build.pk, record=False, @@ -89,7 +90,8 @@ def test_update_docs_unexpected_setup_exception(self, mock_update_build, mock_se build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - result = tasks.update_docs.delay( + update_docs = tasks.UpdateDocsTask() + result = update_docs.delay( self.project.pk, build_pk=build.pk, record=False, @@ -107,7 +109,8 @@ def test_update_docs_unexpected_build_exception(self, mock_update_build, mock_bu build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - result = tasks.update_docs.delay( + update_docs = tasks.UpdateDocsTask() + result = update_docs.delay( self.project.pk, build_pk=build.pk, record=False, diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index b48795e8807..dc3424b93e8 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -18,12 +18,12 @@ def setUp(self): self.project = get(Project, container_time_limit=None) self.version = get(Version, project=self.project) - @mock.patch('readthedocs.projects.tasks.update_docs') + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') def test_trigger_build_time_limit(self, update_docs): """Pass of time limit""" trigger_build(project=self.project, version=self.version) - update_docs.assert_has_calls([ - mock.call.apply_async( + update_docs().apply_async.assert_has_calls([ + mock.call( time_limit=720, soft_time_limit=600, queue=mock.ANY, @@ -38,13 +38,13 @@ def test_trigger_build_time_limit(self, update_docs): ) ]) - @mock.patch('readthedocs.projects.tasks.update_docs') + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') def test_trigger_build_invalid_time_limit(self, update_docs): """Time limit as string""" self.project.container_time_limit = '200s' trigger_build(project=self.project, version=self.version) - update_docs.assert_has_calls([ - mock.call.apply_async( + update_docs().apply_async.assert_has_calls([ + mock.call( time_limit=720, soft_time_limit=600, queue=mock.ANY, @@ -59,13 +59,13 @@ def test_trigger_build_invalid_time_limit(self, update_docs): ) ]) - @mock.patch('readthedocs.projects.tasks.update_docs') + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') def test_trigger_build_rounded_time_limit(self, update_docs): """Time limit should round down""" self.project.container_time_limit = 3 trigger_build(project=self.project, version=self.version) - update_docs.assert_has_calls([ - mock.call.apply_async( + update_docs().apply_async.assert_has_calls([ + mock.call( time_limit=3, soft_time_limit=3, queue=mock.ANY, diff --git a/readthedocs/settings/base.py b/readthedocs/settings/base.py index d595f49e430..a46ccaefa75 100644 --- a/readthedocs/settings/base.py +++ b/readthedocs/settings/base.py @@ -4,8 +4,6 @@ from __future__ import absolute_import import os -import djcelery - from readthedocs.core.settings import Settings try: @@ -15,8 +13,6 @@ donate = False -djcelery.setup_loader() - _ = gettext = lambda s: s @@ -85,9 +81,6 @@ def INSTALLED_APPS(self): # noqa 'django_extensions', 'messages_extends', - # Celery bits - 'djcelery', - # daniellindsleyrocksdahouse 'haystack', 'tastypie', diff --git a/readthedocs/worker.py b/readthedocs/worker.py new file mode 100644 index 00000000000..8febe7c4ccd --- /dev/null +++ b/readthedocs/worker.py @@ -0,0 +1,19 @@ +"""Celery worker application instantiation""" + +from __future__ import absolute_import, unicode_literals + +import os + +from celery import Celery +from django.conf import settings + + +def create_application(): + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'readthedocs.settings.dev') + app = Celery('readthedocs') + app.config_from_object('django.conf:settings') + app.autodiscover_tasks() + return app + + +app = create_application() diff --git a/requirements/pip.txt b/requirements/pip.txt index caa6b917067..2a56390d8ae 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -26,9 +26,8 @@ lxml==3.3.5 defusedxml==0.5.0 # Basic tools -redis==2.10.3 -celery==3.1.23 -django-celery==3.2.1 +redis==2.10.6 +celery==4.1.0 django-allauth==0.32.0 dnspython==1.15.0 From b0c3ad524a94cfca5daf26306fbe9cc218edd9c7 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Fri, 3 Nov 2017 17:22:41 -0600 Subject: [PATCH 02/11] Update docs mentioning Celery --- docs/faq.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/faq.rst b/docs/faq.rst index c6ad95f4690..0410cc65a13 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -104,7 +104,7 @@ If you add a subproject to a project, that documentation will also be served under the parent project's subdomain. For example, -Kombu is a subproject of celery, +Kombu is a subproject of Celery, so you can access it on the `celery.readthedocs.io` domain: http://celery.readthedocs.io/projects/kombu/en/latest/ @@ -204,4 +204,4 @@ file* field. What commit of Read the Docs is in production? ---------------------------------------------- -We deploy readthedocs.org from the `rel` branch in our GitHub repository. You can see the latest commits that have been deployed by looking on GitHub: https://github.com/rtfd/readthedocs.org/commits/rel \ No newline at end of file +We deploy readthedocs.org from the `rel` branch in our GitHub repository. You can see the latest commits that have been deployed by looking on GitHub: https://github.com/rtfd/readthedocs.org/commits/rel From 8f9838aacc32efe4e23d33c37b7ec6b6b0c9f9a6 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 8 Nov 2017 16:57:27 -0700 Subject: [PATCH 03/11] Goof on package name --- readthedocs/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/readthedocs/__init__.py b/readthedocs/__init__.py index 9f175088761..a3e40fa100f 100644 --- a/readthedocs/__init__.py +++ b/readthedocs/__init__.py @@ -1 +1,2 @@ -from .tasks import app +# Import the Celery application before anything else happens +from .worker import app From 0d4166e83e1296260bc1fb5a23ba405176f6f487 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 8 Nov 2017 17:11:48 -0700 Subject: [PATCH 04/11] Missed djcelery import --- readthedocs/projects/tasks.py | 1 - 1 file changed, 1 deletion(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 3ca5b776ac0..8a3124c6ce6 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -21,7 +21,6 @@ from django.conf import settings from django.core.urlresolvers import reverse from django.utils.translation import ugettext_lazy as _ -from djcelery import celery as celery_app from readthedocs_build.config import ConfigError from slumber.exceptions import HttpClientError From 8f75a8545cbcc2ca2cdda6f513ffaa0cc70f7d8c Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 8 Nov 2017 17:18:52 -0700 Subject: [PATCH 05/11] Fix rebased task that was missed --- readthedocs/projects/tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 8a3124c6ce6..9f7c51620e9 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -977,7 +977,7 @@ def clear_html_artifacts(version): remove_dir(version.project.rtd_build_path(version=version.slug)) -@task(queue='web') +@shared_task(queue='web') def sync_callback(_, version_pk, commit, *args, **kwargs): """ This will be called once the sync_files tasks are done. From 63693a3a99bf79289b7ec73508e3aa0f4f569360 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 8 Nov 2017 18:41:42 -0700 Subject: [PATCH 06/11] Handle change to Celery group calls in 4.x --- readthedocs/core/utils/__init__.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index f42b31bb6a4..5293b0a5ed0 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -49,9 +49,15 @@ def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable= task_sig = task.s(*args, **kwargs).set(queue=server) tasks.append(task_sig) if callback: - task_promise = chord(tasks)(callback).get() + task_promise = chord(tasks)(callback).apply_async() else: - task_promise = group(*tasks).apply_async() + # Celery's Group class does some special handling when an iterable with + # len() == 1 is passed in. This will be hit if there is only one server + # defined in the above queue lists + if len(tasks) > 1: + task_promise = group(*tasks).apply_async() + else: + task_promise = group(tasks).apply_async() return task_promise From 3bf9275d5c204a47a7104e880ec625e03d28744c Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Wed, 8 Nov 2017 23:14:07 -0700 Subject: [PATCH 07/11] Fix tests --- .../rtd_tests/tests/projects/test_admin_actions.py | 9 +++++---- readthedocs/rtd_tests/tests/test_privacy_urls.py | 13 ++++++++----- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py index b0f5526b331..7bda4501bf7 100644 --- a/readthedocs/rtd_tests/tests/projects/test_admin_actions.py +++ b/readthedocs/rtd_tests/tests/projects/test_admin_actions.py @@ -57,9 +57,10 @@ def test_project_ban_multiple_owners(self): self.assertFalse(self.project.users.filter(profile__banned=True).exists()) self.assertEqual(self.project.users.filter(profile__banned=False).count(), 2) - @mock.patch('readthedocs.projects.admin.remove_dir') - def test_project_delete(self, remove_dir): + @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 action_data = { ACTION_CHECKBOX_NAME: [self.project.pk], 'action': 'delete_selected', @@ -71,8 +72,8 @@ def test_project_delete(self, remove_dir): action_data ) self.assertFalse(Project.objects.filter(pk=self.project.pk).exists()) - remove_dir.s.assert_has_calls([ + broadcast.assert_has_calls([ mock.call( - self.project.doc_path, + type='app', task=remove_dir, args=[self.project.doc_path] ), ]) diff --git a/readthedocs/rtd_tests/tests/test_privacy_urls.py b/readthedocs/rtd_tests/tests/test_privacy_urls.py index b9fa7c0eaeb..e992a55ae1c 100644 --- a/readthedocs/rtd_tests/tests/test_privacy_urls.py +++ b/readthedocs/rtd_tests/tests/test_privacy_urls.py @@ -1,22 +1,23 @@ from __future__ import absolute_import from __future__ import print_function -from builtins import object import re +from builtins import object from django.contrib.admindocs.views import extract_views_from_urlpatterns from django.test import TestCase from django.core.urlresolvers import reverse +from django_dynamic_fixture import get +import mock +from taggit.models import Tag from readthedocs.builds.models import Build, VersionAlias, BuildCommandResult from readthedocs.comments.models import DocumentComment, NodeSnapshot +from readthedocs.core.utils.tasks import TaskNoPermission from readthedocs.integrations.models import HttpExchange, Integration from readthedocs.projects.models import Project, Domain from readthedocs.oauth.models import RemoteRepository, RemoteOrganization from readthedocs.rtd_tests.utils import create_user -from django_dynamic_fixture import get -from taggit.models import Tag - class URLAccessMixin(object): @@ -341,8 +342,10 @@ def setUp(self): class APIUnauthAccessTest(APIMixin, TestCase): - def test_api_urls(self): + @mock.patch('readthedocs.restapi.views.task_views.get_public_task_data') + def test_api_urls(self, get_public_task_data): from readthedocs.restapi.urls import urlpatterns + get_public_task_data.side_effect = TaskNoPermission('Nope') self._test_url(urlpatterns) def login(self): From 5213ddd2c59a84669d1109f62fb6cdd418f06f33 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 9 Nov 2017 00:09:08 -0700 Subject: [PATCH 08/11] Fix up some linting issues and problems starting the application --- readthedocs/__init__.py | 4 +++- readthedocs/projects/tasks.py | 3 --- readthedocs/worker.py | 12 ++++++------ 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/readthedocs/__init__.py b/readthedocs/__init__.py index a3e40fa100f..1343a18d341 100644 --- a/readthedocs/__init__.py +++ b/readthedocs/__init__.py @@ -1,2 +1,4 @@ +"""Read the Docs""" + # Import the Celery application before anything else happens -from .worker import app +from readthedocs.worker import app # noqa diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 9f7c51620e9..5314dc86b60 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -490,9 +490,6 @@ def send_notifications(self): send_notifications.delay(self.version.pk, build_pk=self.build['id']) -app.register_task(UpdateDocsTask()) - - @shared_task() def update_imported_docs(version_pk): """ diff --git a/readthedocs/worker.py b/readthedocs/worker.py index 8febe7c4ccd..e04d30d6dbb 100644 --- a/readthedocs/worker.py +++ b/readthedocs/worker.py @@ -5,15 +5,15 @@ import os from celery import Celery -from django.conf import settings def create_application(): + """Create a Celery application using Django settings""" os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'readthedocs.settings.dev') - app = Celery('readthedocs') - app.config_from_object('django.conf:settings') - app.autodiscover_tasks() - return app + application = Celery('readthedocs') + application.config_from_object('django.conf:settings') + application.autodiscover_tasks(None) + return application -app = create_application() +app = create_application() # pylint: disable=invalid-name From 4af1767421991934cc54a072d9b2254b1f096202 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 9 Nov 2017 10:53:01 -0700 Subject: [PATCH 09/11] Fix celery task registration --- readthedocs/projects/__init__.py | 1 + readthedocs/projects/apps.py | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 readthedocs/projects/apps.py diff --git a/readthedocs/projects/__init__.py b/readthedocs/projects/__init__.py index e69de29bb2d..ff5ded49b17 100644 --- a/readthedocs/projects/__init__.py +++ b/readthedocs/projects/__init__.py @@ -0,0 +1 @@ +default_app_config = 'readthedocs.projects.apps.ProjectsConfig' diff --git a/readthedocs/projects/apps.py b/readthedocs/projects/apps.py new file mode 100644 index 00000000000..71111f4e8d0 --- /dev/null +++ b/readthedocs/projects/apps.py @@ -0,0 +1,12 @@ +"""Project app config""" + +from django.apps import AppConfig + + +class ProjectsConfig(AppConfig): + name = 'readthedocs.projects' + + def ready(self): + from readthedocs.projects.tasks import UpdateDocsTask + from readthedocs.worker import app + app.tasks.register(UpdateDocsTask) From f20641b0500ef224ced417b760555562ef2a7066 Mon Sep 17 00:00:00 2001 From: Anthony Johnson Date: Thu, 9 Nov 2017 11:38:31 -0700 Subject: [PATCH 10/11] We don't need shared_task anymore, swap for readthedocs.worker.app.task --- readthedocs/core/tasks.py | 5 +++-- readthedocs/projects/tasks.py | 36 +++++++++++++++++------------------ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/readthedocs/core/tasks.py b/readthedocs/core/tasks.py index b3efbab305c..e5b67355e3f 100644 --- a/readthedocs/core/tasks.py +++ b/readthedocs/core/tasks.py @@ -3,19 +3,20 @@ from __future__ import absolute_import import logging -from celery import shared_task from django.conf import settings from django.core.mail import EmailMultiAlternatives from django.template.loader import get_template from django.template import TemplateDoesNotExist +from readthedocs.worker import app + log = logging.getLogger(__name__) EMAIL_TIME_LIMIT = 30 -@shared_task(queue='web', time_limit=EMAIL_TIME_LIMIT) +@app.task(queue='web', time_limit=EMAIL_TIME_LIMIT) def send_email_task(recipient, subject, template, template_html, context=None): """Send multipart email diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 5314dc86b60..68ca3ca2a7d 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -16,7 +16,7 @@ import requests from builtins import str -from celery import shared_task, Task +from celery import Task from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings from django.core.urlresolvers import reverse @@ -490,7 +490,7 @@ def send_notifications(self): send_notifications.delay(self.version.pk, build_pk=self.build['id']) -@shared_task() +@app.task() def update_imported_docs(version_pk): """ Check out or update the given project's repository @@ -569,7 +569,7 @@ def update_imported_docs(version_pk): # Web tasks -@shared_task(queue='web') +@app.task(queue='web') def sync_files(project_pk, version_pk, hostname=None, html=False, localmedia=False, search=False, pdf=False, epub=False): """Sync build artifacts to application instances @@ -601,7 +601,7 @@ def sync_files(project_pk, version_pk, hostname=None, html=False, update_static_metadata(project_pk) -@shared_task(queue='web') +@app.task(queue='web') def move_files(version_pk, hostname, html=False, localmedia=False, search=False, pdf=False, epub=False): """Task to move built documentation to web servers @@ -667,7 +667,7 @@ def move_files(version_pk, hostname, html=False, localmedia=False, search=False, Syncer.copy(from_path, to_path, host=hostname) -@shared_task(queue='web') +@app.task(queue='web') def update_search(version_pk, commit, delete_non_commit_files=True): """Task to update search indexes @@ -702,7 +702,7 @@ def update_search(version_pk, commit, delete_non_commit_files=True): ) -@shared_task(queue='web') +@app.task(queue='web') def symlink_project(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -710,7 +710,7 @@ def symlink_project(project_pk): sym.run() -@shared_task(queue='web') +@app.task(queue='web') def symlink_domain(project_pk, domain_pk, delete=False): project = Project.objects.get(pk=project_pk) domain = Domain.objects.get(pk=domain_pk) @@ -722,7 +722,7 @@ def symlink_domain(project_pk, domain_pk, delete=False): sym.symlink_cnames(domain) -@shared_task(queue='web') +@app.task(queue='web') def symlink_subproject(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -730,7 +730,7 @@ def symlink_subproject(project_pk): sym.symlink_subprojects() -@shared_task(queue='web') +@app.task(queue='web') def fileify(version_pk, commit): """ Create ImportedFile objects for all of a version's files. @@ -806,7 +806,7 @@ def _manage_imported_files(version, path, commit): purge(cdn_ids[version.project.slug], changed_files) -@shared_task(queue='web') +@app.task(queue='web') def send_notifications(version_pk, build_pk): version = Version.objects.get(pk=version_pk) build = Build.objects.get(pk=build_pk) @@ -875,7 +875,7 @@ def webhook_notification(version, build, hook_url): requests.post(hook_url, data=data) -@shared_task(queue='web') +@app.task(queue='web') def update_static_metadata(project_pk, path=None): """Update static metadata JSON file @@ -921,7 +921,7 @@ def update_static_metadata(project_pk, path=None): # Random Tasks -@shared_task() +@app.task() def remove_dir(path): """ Remove a directory on the build/celery server. @@ -933,7 +933,7 @@ def remove_dir(path): shutil.rmtree(path, ignore_errors=True) -@shared_task() +@app.task() def clear_artifacts(version_pk): """Remove artifacts from the web servers""" version = Version.objects.get(pk=version_pk) @@ -943,7 +943,7 @@ def clear_artifacts(version_pk): clear_html_artifacts(version) -@shared_task() +@app.task() def clear_pdf_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -951,7 +951,7 @@ def clear_pdf_artifacts(version): type_='pdf', version_slug=version.slug)) -@shared_task() +@app.task() def clear_epub_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -959,7 +959,7 @@ def clear_epub_artifacts(version): type_='epub', version_slug=version.slug)) -@shared_task() +@app.task() def clear_htmlzip_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -967,14 +967,14 @@ def clear_htmlzip_artifacts(version): type_='htmlzip', version_slug=version.slug)) -@shared_task() +@app.task() def clear_html_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) remove_dir(version.project.rtd_build_path(version=version.slug)) -@shared_task(queue='web') +@app.task(queue='web') def sync_callback(_, version_pk, commit, *args, **kwargs): """ This will be called once the sync_files tasks are done. From 99a9aff252d464a178ed88a949d7d0e75642f64d Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Fri, 10 Nov 2017 15:06:14 -0700 Subject: [PATCH 11/11] Fix call to chord --- readthedocs/core/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 5293b0a5ed0..80e5d90631b 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -49,7 +49,7 @@ def broadcast(type, task, args, kwargs=None, callback=None): # pylint: disable= task_sig = task.s(*args, **kwargs).set(queue=server) tasks.append(task_sig) if callback: - task_promise = chord(tasks)(callback).apply_async() + task_promise = chord(tasks, callback).apply_async() else: # Celery's Group class does some special handling when an iterable with # len() == 1 is passed in. This will be hit if there is only one server