From d873986be41694f456f1deac60146e3d4a250a7b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Tue, 21 Aug 2018 18:08:21 -0300 Subject: [PATCH 1/2] Route task to proper queue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #4033, I introduce a bug when using Celery signatures. From Celery's docs at http://docs.celeryproject.org/en/latest/reference/celery.html#celery.signature > the .s() shortcut does not allow you to specify execution options but there’s a chaning .set method that returns the signature So, instead of dealing with multiple `.set()`, I'm just using the `.signature()` method of the task which is more explicit. --- readthedocs/core/utils/__init__.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/readthedocs/core/utils/__init__.py b/readthedocs/core/utils/__init__.py index 6f3e83997cf..82ab13dbf11 100644 --- a/readthedocs/core/utils/__init__.py +++ b/readthedocs/core/utils/__init__.py @@ -139,13 +139,12 @@ def prepare_build( options['time_limit'] = int(time_limit * 1.2) update_docs_task = UpdateDocsTask() - - # Py 2.7 doesn't support ``**`` expand syntax twice. We create just one big - # kwargs (including the options) for this and expand it just once. - # return update_docs_task.si(project.pk, **kwargs, **options) - kwargs.update(options) - - return update_docs_task.si(project.pk, **kwargs) + return update_docs_task.signature( + (project.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ) def trigger_build(project, version=None, record=True, force=False): From 08a5a91bd41b7586b0d362c67b44e2bcda80133b Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Wed, 22 Aug 2018 10:23:08 -0300 Subject: [PATCH 2/2] Add test for custom celery queue on UpdateDocsTask and fix others --- .../rtd_tests/tests/test_core_utils.py | 109 +++++++++++++----- 1 file changed, 78 insertions(+), 31 deletions(-) diff --git a/readthedocs/rtd_tests/tests/test_core_utils.py b/readthedocs/rtd_tests/tests/test_core_utils.py index a009982b723..b17e5c1f59d 100644 --- a/readthedocs/rtd_tests/tests/test_core_utils.py +++ b/readthedocs/rtd_tests/tests/test_core_utils.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- """Test core util functions""" from __future__ import absolute_import @@ -17,62 +18,108 @@ def setUp(self): self.project = get(Project, container_time_limit=None) self.version = get(Version, project=self.project) + @mock.patch('readthedocs.projects.tasks.UpdateDocsTask') + def test_trigger_custom_queue(self, update_docs): + """Use a custom queue when routing the task""" + self.project.build_queue = 'build03' + trigger_build(project=self.project, version=self.version) + kwargs = { + 'version_pk': self.version.pk, + 'record': True, + 'force': False, + 'build_pk': mock.ANY, + } + options = { + 'queue': 'build03', + 'time_limit': 720, + 'soft_time_limit': 600, + } + update_docs().signature.assert_has_calls([ + mock.call( + (self.project.pk,), + kwargs=kwargs, + options=options, + immutable=True, + ), + ]) + update_docs().signature().apply_async.assert_called() + @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().si.assert_has_calls([ + kwargs = { + 'version_pk': self.version.pk, + 'record': True, + 'force': False, + 'build_pk': mock.ANY, + } + options = { + 'queue': mock.ANY, + 'time_limit': 720, + 'soft_time_limit': 600, + } + update_docs().signature.assert_has_calls([ mock.call( - self.project.pk, - time_limit=720, - soft_time_limit=600, - queue=mock.ANY, - force=False, - record=True, - build_pk=mock.ANY, - version_pk=self.version.id, + (self.project.pk,), + kwargs=kwargs, + options=options, + immutable=True, ), ]) - update_docs().si().apply_async.assert_called() + update_docs().signature().apply_async.assert_called() @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().si.assert_has_calls([ + kwargs = { + 'version_pk': self.version.pk, + 'record': True, + 'force': False, + 'build_pk': mock.ANY, + } + options = { + 'queue': mock.ANY, + 'time_limit': 720, + 'soft_time_limit': 600, + } + update_docs().signature.assert_has_calls([ mock.call( - self.project.pk, - time_limit=720, - soft_time_limit=600, - queue=mock.ANY, - force=False, - record=True, - build_pk=mock.ANY, - version_pk=self.version.id, + (self.project.pk,), + kwargs=kwargs, + options=options, + immutable=True, ), ]) - update_docs().si().apply_async.assert_called() + update_docs().signature().apply_async.assert_called() @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().si.assert_has_calls([ + kwargs = { + 'version_pk': self.version.pk, + 'record': True, + 'force': False, + 'build_pk': mock.ANY, + } + options = { + 'queue': mock.ANY, + 'time_limit': 3, + 'soft_time_limit': 3, + } + update_docs().signature.assert_has_calls([ mock.call( - self.project.pk, - time_limit=3, - soft_time_limit=3, - queue=mock.ANY, - force=False, - record=True, - build_pk=mock.ANY, - version_pk=self.version.id, + (self.project.pk,), + kwargs=kwargs, + options=options, + immutable=True, ), ]) - update_docs().si().apply_async.assert_called() - + update_docs().signature().apply_async.assert_called() def test_slugify(self): """Test additional slugify"""