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 diff --git a/readthedocs/__init__.py b/readthedocs/__init__.py index e69de29bb2d..1343a18d341 100644 --- a/readthedocs/__init__.py +++ b/readthedocs/__init__.py @@ -0,0 +1,4 @@ +"""Read the Docs""" + +# Import the Celery application before anything else happens +from readthedocs.worker import app # noqa 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..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 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 -@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/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index fca8e13f96f..80e5d90631b 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 @@ -77,7 +83,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 +127,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/__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) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index a52fa80c3af..68ca3ca2a7d 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -16,12 +16,11 @@ import requests from builtins import str -from celery import task, Task +from celery import Task from celery.exceptions import SoftTimeLimitExceeded 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 @@ -53,6 +52,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 +80,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 +490,7 @@ def send_notifications(self): send_notifications.delay(self.version.pk, build_pk=self.build['id']) -update_docs = celery_app.tasks[UpdateDocsTask.name] - - -@task() +@app.task() def update_imported_docs(version_pk): """ Check out or update the given project's repository @@ -572,7 +569,7 @@ def update_imported_docs(version_pk): # Web tasks -@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 @@ -604,7 +601,7 @@ def sync_files(project_pk, version_pk, hostname=None, html=False, update_static_metadata(project_pk) -@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 @@ -670,7 +667,7 @@ def move_files(version_pk, hostname, html=False, localmedia=False, search=False, Syncer.copy(from_path, to_path, host=hostname) -@task(queue='web') +@app.task(queue='web') def update_search(version_pk, commit, delete_non_commit_files=True): """Task to update search indexes @@ -705,7 +702,7 @@ def update_search(version_pk, commit, delete_non_commit_files=True): ) -@task(queue='web') +@app.task(queue='web') def symlink_project(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -713,7 +710,7 @@ def symlink_project(project_pk): sym.run() -@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) @@ -725,7 +722,7 @@ def symlink_domain(project_pk, domain_pk, delete=False): sym.symlink_cnames(domain) -@task(queue='web') +@app.task(queue='web') def symlink_subproject(project_pk): project = Project.objects.get(pk=project_pk) for symlink in [PublicSymlink, PrivateSymlink]: @@ -733,7 +730,7 @@ def symlink_subproject(project_pk): sym.symlink_subprojects() -@task(queue='web') +@app.task(queue='web') def fileify(version_pk, commit): """ Create ImportedFile objects for all of a version's files. @@ -809,7 +806,7 @@ def _manage_imported_files(version, path, commit): purge(cdn_ids[version.project.slug], changed_files) -@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) @@ -878,7 +875,7 @@ def webhook_notification(version, build, hook_url): requests.post(hook_url, data=data) -@task(queue='web') +@app.task(queue='web') def update_static_metadata(project_pk, path=None): """Update static metadata JSON file @@ -924,7 +921,7 @@ def update_static_metadata(project_pk, path=None): # Random Tasks -@task() +@app.task() def remove_dir(path): """ Remove a directory on the build/celery server. @@ -936,7 +933,7 @@ def remove_dir(path): shutil.rmtree(path, ignore_errors=True) -@task() +@app.task() def clear_artifacts(version_pk): """Remove artifacts from the web servers""" version = Version.objects.get(pk=version_pk) @@ -946,7 +943,7 @@ def clear_artifacts(version_pk): clear_html_artifacts(version) -@task() +@app.task() def clear_pdf_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -954,7 +951,7 @@ def clear_pdf_artifacts(version): type_='pdf', version_slug=version.slug)) -@task() +@app.task() def clear_epub_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -962,7 +959,7 @@ def clear_epub_artifacts(version): type_='epub', version_slug=version.slug)) -@task() +@app.task() def clear_htmlzip_artifacts(version): if isinstance(version, int): version = Version.objects.get(pk=version) @@ -970,14 +967,14 @@ def clear_htmlzip_artifacts(version): type_='htmlzip', version_slug=version.slug)) -@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)) -@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. 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_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/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): 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..e04d30d6dbb --- /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 + + +def create_application(): + """Create a Celery application using Django settings""" + os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'readthedocs.settings.dev') + application = Celery('readthedocs') + application.config_from_object('django.conf:settings') + application.autodiscover_tasks(None) + return application + + +app = create_application() # pylint: disable=invalid-name 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