diff --git a/readthedocs/core/management/commands/pull.py b/readthedocs/core/management/commands/pull.py index c6c8d783043..3540a35c077 100644 --- a/readthedocs/core/management/commands/pull.py +++ b/readthedocs/core/management/commands/pull.py @@ -20,6 +20,4 @@ def handle(self, *args, **options): if args: for slug in args: version = utils.version_from_slug(slug, LATEST) - tasks.SyncRepositoryTask().run( - version.pk, - ) + tasks.sync_repository_task(version.pk) diff --git a/readthedocs/core/management/commands/update_api.py b/readthedocs/core/management/commands/update_api.py index e8433513bdd..e95b23b1899 100644 --- a/readthedocs/core/management/commands/update_api.py +++ b/readthedocs/core/management/commands/update_api.py @@ -33,5 +33,5 @@ def handle(self, *args, **options): project_data = api.project(slug).get() p = APIProject(**project_data) log.info("Building %s", p) - update_docs = tasks.UpdateDocsTask() - update_docs.run(pk=p.pk, docker=docker) + # pylint: disable=no-value-for-parameter + tasks.update_docs_task(p.pk, docker=docker) diff --git a/readthedocs/core/management/commands/update_repos.py b/readthedocs/core/management/commands/update_repos.py index 2dbd0268bd3..b5233d2d6df 100644 --- a/readthedocs/core/management/commands/update_repos.py +++ b/readthedocs/core/management/commands/update_repos.py @@ -73,8 +73,9 @@ def handle(self, *args, **options): state='triggered', ) - tasks.UpdateDocsTask().run( - pk=version.project_id, + # pylint: disable=no-value-for-parameter + tasks.update_docs_task( + version.project_id, build_pk=build.pk, version_pk=version.pk, ) @@ -89,15 +90,17 @@ def handle(self, *args, **options): active=True, uploaded=False, ): - tasks.UpdateDocsTask().run( - pk=version.project_id, + # pylint: disable=no-value-for-parameter + tasks.update_docs_task( + version.project_id, force=force, version_pk=version.pk, ) else: log.info('Updating all docs') for project in Project.objects.all(): - tasks.UpdateDocsTask().run( - pk=project.pk, + # pylint: disable=no-value-for-parameter + tasks.update_docs_task( + project.pk, force=force, ) diff --git a/readthedocs/core/management/commands/update_versions.py b/readthedocs/core/management/commands/update_versions.py index c3ae6fa8e1b..8961d6706cf 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 UpdateDocsTask +from readthedocs.projects.tasks import update_docs_task class Command(BaseCommand): @@ -13,6 +13,9 @@ 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) + # pylint: disable=no-value-for-parameter + update_docs_task( + version.project_id, + record=False, + version_pk=version.pk + ) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 82ab13dbf11..c4e33769690 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -89,10 +89,10 @@ def prepare_build( :param record: whether or not record the build in a new Build object :param force: build the HTML documentation even if the files haven't changed :param immutable: whether or not create an immutable Celery signature - :returns: Celery signature of UpdateDocsTask to be executed + :returns: Celery signature of update_docs_task to be executed """ # Avoid circular import - from readthedocs.projects.tasks import UpdateDocsTask + from readthedocs.projects.tasks import update_docs_task from readthedocs.builds.models import Build if project.skip: @@ -138,9 +138,8 @@ def prepare_build( options['soft_time_limit'] = time_limit options['time_limit'] = int(time_limit * 1.2) - update_docs_task = UpdateDocsTask() return update_docs_task.signature( - (project.pk,), + args=(project.pk,), kwargs=kwargs, options=options, immutable=True, diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 253d8848ef3..c788d6a2e7b 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -12,7 +12,7 @@ from readthedocs.builds.constants import LATEST from readthedocs.projects import constants from readthedocs.projects.models import Project, Feature -from readthedocs.projects.tasks import SyncRepositoryTask +from readthedocs.projects.tasks import sync_repository_task import logging @@ -128,12 +128,9 @@ def _build_url(url, projects, branches): for project in projects: (built, not_building) = build_branches(project, branches) if not built: - # Call SyncRepositoryTask to update tag/branch info + # Call sync_repository_task to update tag/branch info version = project.versions.get(slug=LATEST) - sync_repository = SyncRepositoryTask() - sync_repository.apply_async( - args=(version.pk,), - ) + sync_repository_task.delay(version.pk) msg = '(URL Build) Syncing versions for %s' % project.slug log.info(msg) all_built[project.slug] = built diff --git a/readthedocs/projects/apps.py b/readthedocs/projects/apps.py index 42c0892b9eb..e29afbe49ce 100644 --- a/readthedocs/projects/apps.py +++ b/readthedocs/projects/apps.py @@ -5,10 +5,3 @@ class ProjectsConfig(AppConfig): name = 'readthedocs.projects' - - def ready(self): - from readthedocs.projects import tasks - from readthedocs.worker import app - - app.tasks.register(tasks.SyncRepositoryTask) - app.tasks.register(tasks.UpdateDocsTask) diff --git a/readthedocs/projects/tasks.py b/readthedocs/projects/tasks.py index 5669be97fbe..7b669ee9c0f 100644 --- a/readthedocs/projects/tasks.py +++ b/readthedocs/projects/tasks.py @@ -21,7 +21,6 @@ import requests from builtins import str -from celery import Task from celery.exceptions import SoftTimeLimitExceeded from django.conf import settings from django.core.urlresolvers import reverse @@ -179,19 +178,11 @@ def _log(self, msg): msg=msg)) -# TODO SyncRepositoryTask should be refactored into a standard celery task, -# there is no more need to have this be a separate class -class SyncRepositoryTask(Task): - +@app.task(max_retries=5, default_retry_delay=7 * 60) +def sync_repository_task(version_pk): """Celery task to trigger VCS version sync.""" - - max_retries = 5 - default_retry_delay = (7 * 60) - name = __name__ + '.sync_repository' - - def run(self, *args, **kwargs): - step = SyncRepositoryTaskStep() - return step.run(*args, **kwargs) + step = SyncRepositoryTaskStep() + return step.run(version_pk) class SyncRepositoryTaskStep(SyncRepositoryMixin): @@ -232,17 +223,10 @@ def run(self, version_pk): # pylint: disable=arguments-differ return False -# TODO UpdateDocsTask should be refactored into a standard celery task, -# there is no more need to have this be a separate class -class UpdateDocsTask(Task): - - max_retries = 5 - default_retry_delay = (7 * 60) - name = __name__ + '.update_docs' - - def run(self, *args, **kwargs): - step = UpdateDocsTaskStep(task=self) - return step.run(*args, **kwargs) +@app.task(bind=True, max_retries=5, default_retry_delay=7 * 60) +def update_docs_task(self, project_id, *args, **kwargs): + step = UpdateDocsTaskStep(task=self) + return step.run(project_id, *args, **kwargs) class UpdateDocsTaskStep(SyncRepositoryMixin): diff --git a/readthedocs/rtd_tests/tests/test_celery.py b/readthedocs/rtd_tests/tests/test_celery.py index f806646dd7f..4e5ed8856e6 100644 --- a/readthedocs/rtd_tests/tests/test_celery.py +++ b/readthedocs/rtd_tests/tests/test_celery.py @@ -76,8 +76,7 @@ def test_update_docs(self): build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - update_docs = tasks.UpdateDocsTask() - result = update_docs.delay( + result = tasks.update_docs_task.delay( self.project.pk, build_pk=build.pk, record=False, @@ -94,8 +93,7 @@ def test_update_docs_unexpected_setup_exception(self, mock_setup_vcs): build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - update_docs = tasks.UpdateDocsTask() - result = update_docs.delay( + result = tasks.update_docs_task.delay( self.project.pk, build_pk=build.pk, record=False, @@ -112,8 +110,7 @@ def test_update_docs_unexpected_build_exception(self, mock_build_docs): build = get(Build, project=self.project, version=self.project.versions.first()) with mock_api(self.repo) as mapi: - update_docs = tasks.UpdateDocsTask() - result = update_docs.delay( + result = tasks.update_docs_task.delay( self.project.pk, build_pk=build.pk, record=False, @@ -123,10 +120,7 @@ def test_update_docs_unexpected_build_exception(self, mock_build_docs): def test_sync_repository(self): version = self.project.versions.get(slug=LATEST) with mock_api(self.repo): - sync_repository = tasks.SyncRepositoryTask() - result = sync_repository.apply_async( - args=(version.pk,), - ) + result = tasks.sync_repository_task.delay(version.pk) self.assertTrue(result.successful()) @patch('readthedocs.projects.tasks.api_v2') diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index b17e5c1f59d..78fb68cb1fd 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -18,7 +18,7 @@ def setUp(self): self.project = get(Project, container_time_limit=None) self.version = get(Version, project=self.project) - @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') + @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_custom_queue(self, update_docs): """Use a custom queue when routing the task""" self.project.build_queue = 'build03' @@ -34,17 +34,17 @@ def test_trigger_custom_queue(self, update_docs): 'time_limit': 720, 'soft_time_limit': 600, } - update_docs().signature.assert_has_calls([ + update_docs.signature.assert_has_calls([ mock.call( - (self.project.pk,), + args=(self.project.pk,), kwargs=kwargs, options=options, immutable=True, ), ]) - update_docs().signature().apply_async.assert_called() + update_docs.signature().apply_async.assert_called() - @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') + @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_time_limit(self, update_docs): """Pass of time limit""" trigger_build(project=self.project, version=self.version) @@ -59,17 +59,17 @@ def test_trigger_build_time_limit(self, update_docs): 'time_limit': 720, 'soft_time_limit': 600, } - update_docs().signature.assert_has_calls([ + update_docs.signature.assert_has_calls([ mock.call( - (self.project.pk,), + args=(self.project.pk,), kwargs=kwargs, options=options, immutable=True, ), ]) - update_docs().signature().apply_async.assert_called() + update_docs.signature().apply_async.assert_called() - @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') + @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_invalid_time_limit(self, update_docs): """Time limit as string""" self.project.container_time_limit = '200s' @@ -85,17 +85,17 @@ def test_trigger_build_invalid_time_limit(self, update_docs): 'time_limit': 720, 'soft_time_limit': 600, } - update_docs().signature.assert_has_calls([ + update_docs.signature.assert_has_calls([ mock.call( - (self.project.pk,), + args=(self.project.pk,), kwargs=kwargs, options=options, immutable=True, ), ]) - update_docs().signature().apply_async.assert_called() + update_docs.signature().apply_async.assert_called() - @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') + @mock.patch('readthedocs.projects.tasks.update_docs_task') def test_trigger_build_rounded_time_limit(self, update_docs): """Time limit should round down""" self.project.container_time_limit = 3 @@ -111,15 +111,15 @@ def test_trigger_build_rounded_time_limit(self, update_docs): 'time_limit': 3, 'soft_time_limit': 3, } - update_docs().signature.assert_has_calls([ + update_docs.signature.assert_has_calls([ mock.call( - (self.project.pk,), + args=(self.project.pk,), kwargs=kwargs, options=options, immutable=True, ), ]) - update_docs().signature().apply_async.assert_called() + update_docs.signature().apply_async.assert_called() def test_slugify(self): """Test additional slugify""" diff --git a/readthedocs/rtd_tests/tests/test_privacy.py b/readthedocs/rtd_tests/tests/test_privacy.py index ac8b15ac98d..60acf941055 100644 --- a/readthedocs/rtd_tests/tests/test_privacy.py +++ b/readthedocs/rtd_tests/tests/test_privacy.py @@ -27,7 +27,7 @@ def setUp(self): self.tester.set_password('test') self.tester.save() - tasks.UpdateDocsTask.delay = mock.Mock() + tasks.update_docs_task.delay = mock.Mock() def _create_kong(self, privacy_level='private', version_privacy_level='private'):