From f4af2810dd2f70718a757f733b43225527f6aa3d Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Wed, 25 May 2022 14:27:26 +0530 Subject: [PATCH 1/7] save extended properties only when asked for --- django_celery_results/backends/database.py | 97 +++++++++++++--------- 1 file changed, 57 insertions(+), 40 deletions(-) diff --git a/django_celery_results/backends/database.py b/django_celery_results/backends/database.py index 1d7e0043..991c8dd8 100644 --- a/django_celery_results/backends/database.py +++ b/django_celery_results/backends/database.py @@ -54,6 +54,52 @@ def exception_safe_to_retry(self, exc): return True return False + def _get_extended_properties(self, request, traceback, using): + properties = getattr(request, 'properties', {}) or {} + extended_props = { + 'periodic_task_name': None, + 'task_args': None, + 'task_kwargs': None, + 'task_name': None, + 'traceback': None, + 'using': None, + 'worker': None, + } + if request and self.app.conf.find_value_for_key('extended', 'result'): + + if getattr(request, 'argsrepr', None) is not None: + # task protocol 2 + task_args = request.argsrepr + else: + # task protocol 1 + task_args = getattr(request, 'args', None) + + if getattr(request, 'kwargsrepr', None) is not None: + # task protocol 2 + task_kwargs = request.kwargsrepr + else: + # task protocol 1 + task_kwargs = getattr(request, 'kwargs', None) + + # Encode input arguments + if task_args is not None: + _, _, task_args = self.encode_content(task_args) + + if task_kwargs is not None: + _, _, task_kwargs = self.encode_content(task_kwargs) + + extended_props.update({ + 'periodic_task_name': properties.get('periodic_task_name', None), + 'task_args': task_kwargs, + 'task_kwargs': task_args, + 'task_name': getattr(request, 'task', None), + 'traceback': traceback, + 'using': using, + 'worker': getattr(request, 'hostname', None), + }) + + return extended_props + def _store_result( self, task_id, @@ -69,48 +115,19 @@ def _store_result( {'children': self.current_task_children(request)} ) - task_name = getattr(request, 'task', None) - properties = getattr(request, 'properties', {}) or {} - periodic_task_name = properties.get('periodic_task_name', None) - worker = getattr(request, 'hostname', None) - - # Get input arguments - if getattr(request, 'argsrepr', None) is not None: - # task protocol 2 - task_args = request.argsrepr - else: - # task protocol 1 - task_args = getattr(request, 'args', None) + task_props = { + 'content_encoding': content_encoding, + 'content_type' : content_type, + 'meta': meta, + 'result': result, + 'status': status, + 'task_id': task_id, + 'traceback': traceback, + } - if getattr(request, 'kwargsrepr', None) is not None: - # task protocol 2 - task_kwargs = request.kwargsrepr - else: - # task protocol 1 - task_kwargs = getattr(request, 'kwargs', None) + task_props.update(self._get_extended_properties(request, traceback, using)) - # Encode input arguments - if task_args is not None: - _, _, task_args = self.encode_content(task_args) - - if task_kwargs is not None: - _, _, task_kwargs = self.encode_content(task_kwargs) - - self.TaskModel._default_manager.store_result( - content_type, - content_encoding, - task_id, - result, - status, - traceback=traceback, - meta=meta, - periodic_task_name=periodic_task_name, - task_name=task_name, - task_args=task_args, - task_kwargs=task_kwargs, - worker=worker, - using=using, - ) + self.TaskModel._default_manager.store_result(**task_props) return result def _get_task_meta_for(self, task_id): From 8fdcfe972c2f0dfbf5c518db934fe0b497d78fd3 Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Wed, 25 May 2022 14:40:31 +0530 Subject: [PATCH 2/7] lint --- django_celery_results/backends/database.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/django_celery_results/backends/database.py b/django_celery_results/backends/database.py index 991c8dd8..f9f66c88 100644 --- a/django_celery_results/backends/database.py +++ b/django_celery_results/backends/database.py @@ -55,7 +55,6 @@ def exception_safe_to_retry(self, exc): return False def _get_extended_properties(self, request, traceback, using): - properties = getattr(request, 'properties', {}) or {} extended_props = { 'periodic_task_name': None, 'task_args': None, @@ -66,7 +65,7 @@ def _get_extended_properties(self, request, traceback, using): 'worker': None, } if request and self.app.conf.find_value_for_key('extended', 'result'): - + if getattr(request, 'argsrepr', None) is not None: # task protocol 2 task_args = request.argsrepr @@ -87,9 +86,11 @@ def _get_extended_properties(self, request, traceback, using): if task_kwargs is not None: _, _, task_kwargs = self.encode_content(task_kwargs) - + + properties = getattr(request, 'properties', {}) or {} + periodic_task_name = properties.get('periodic_task_name', None) extended_props.update({ - 'periodic_task_name': properties.get('periodic_task_name', None), + 'periodic_task_name': periodic_task_name, 'task_args': task_kwargs, 'task_kwargs': task_args, 'task_name': getattr(request, 'task', None), @@ -97,7 +98,7 @@ def _get_extended_properties(self, request, traceback, using): 'using': using, 'worker': getattr(request, 'hostname', None), }) - + return extended_props def _store_result( @@ -117,15 +118,17 @@ def _store_result( task_props = { 'content_encoding': content_encoding, - 'content_type' : content_type, + 'content_type': content_type, 'meta': meta, 'result': result, - 'status': status, + 'status': status, 'task_id': task_id, 'traceback': traceback, } - task_props.update(self._get_extended_properties(request, traceback, using)) + task_props.update( + self._get_extended_properties(request, traceback, using) + ) self.TaskModel._default_manager.store_result(**task_props) return result From 6f757fe4810d8d1813f70ae91427ffcdf9823b0c Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Thu, 26 May 2022 12:14:09 +0530 Subject: [PATCH 3/7] bugfix: args and kwargs swapped --- django_celery_results/backends/database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/django_celery_results/backends/database.py b/django_celery_results/backends/database.py index f9f66c88..b976e225 100644 --- a/django_celery_results/backends/database.py +++ b/django_celery_results/backends/database.py @@ -91,8 +91,8 @@ def _get_extended_properties(self, request, traceback, using): periodic_task_name = properties.get('periodic_task_name', None) extended_props.update({ 'periodic_task_name': periodic_task_name, - 'task_args': task_kwargs, - 'task_kwargs': task_args, + 'task_args': task_args, + 'task_kwargs': task_kwargs, 'task_name': getattr(request, 'task', None), 'traceback': traceback, 'using': using, From 6974a76a02152cb2bbe13edc2957c400d67f35ee Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Thu, 26 May 2022 12:14:52 +0530 Subject: [PATCH 4/7] update: conf value to make previous tests happy --- t/unit/backends/test_database.py | 1 + 1 file changed, 1 insertion(+) diff --git a/t/unit/backends/test_database.py b/t/unit/backends/test_database.py index 99b97106..5eda89eb 100644 --- a/t/unit/backends/test_database.py +++ b/t/unit/backends/test_database.py @@ -31,6 +31,7 @@ def setup_backend(self): self.app.conf.result_serializer = 'json' self.app.conf.result_backend = ( 'django_celery_results.backends:DatabaseBackend') + self.app.conf.result_extended = True self.b = DatabaseBackend(app=self.app) def _create_request(self, task_id, name, args, kwargs, From 7edbd42250f751b654108fa55e09ba714d4b6efd Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Thu, 26 May 2022 12:32:40 +0530 Subject: [PATCH 5/7] add: tests to verify result_extended flag works --- t/unit/backends/test_database.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/t/unit/backends/test_database.py b/t/unit/backends/test_database.py index 5eda89eb..53dda16c 100644 --- a/t/unit/backends/test_database.py +++ b/t/unit/backends/test_database.py @@ -860,3 +860,30 @@ def test_groupresult_save_restore_nested(self): restored_group = self.b.restore_group(group_id=group_id) assert restored_group == group + + def test_backend_result_extended_is_false(self): + self.app.conf.result_extended = False + self.b = DatabaseBackend(app=self.app) + tid2 = uuid() + request = self._create_request( + task_id=tid2, + name='my_task', + args=['a', 1, True], + kwargs={'c': 6, 'd': 'e', 'f': False}, + ) + result = 'foo' + + self.b.mark_as_done(tid2, result, request=request) + + mindb = self.b.get_task_meta(tid2) + + # check meta data + assert mindb.get('result') == 'foo' + assert mindb.get('task_name') is None + assert mindb.get('task_args') is None + assert mindb.get('task_kwargs') is None + + # check task_result object + tr = TaskResult.objects.get(task_id=tid2) + assert tr.task_args is None + assert tr.task_kwargs is None From 8d2757f9d0a7996a9a647c636c7473a7f12af7ec Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Fri, 10 Jun 2022 18:20:43 +0530 Subject: [PATCH 6/7] move: using attr from extended properties --- django_celery_results/backends/database.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/django_celery_results/backends/database.py b/django_celery_results/backends/database.py index b976e225..35fb94b2 100644 --- a/django_celery_results/backends/database.py +++ b/django_celery_results/backends/database.py @@ -54,14 +54,13 @@ def exception_safe_to_retry(self, exc): return True return False - def _get_extended_properties(self, request, traceback, using): + def _get_extended_properties(self, request, traceback): extended_props = { 'periodic_task_name': None, 'task_args': None, 'task_kwargs': None, 'task_name': None, 'traceback': None, - 'using': None, 'worker': None, } if request and self.app.conf.find_value_for_key('extended', 'result'): @@ -95,7 +94,6 @@ def _get_extended_properties(self, request, traceback, using): 'task_kwargs': task_kwargs, 'task_name': getattr(request, 'task', None), 'traceback': traceback, - 'using': using, 'worker': getattr(request, 'hostname', None), }) @@ -124,10 +122,11 @@ def _store_result( 'status': status, 'task_id': task_id, 'traceback': traceback, + 'using': using, } task_props.update( - self._get_extended_properties(request, traceback, using) + self._get_extended_properties(request, traceback) ) self.TaskModel._default_manager.store_result(**task_props) From af09b8784cebb1014dd7a0b924168a84dfab1bf1 Mon Sep 17 00:00:00 2001 From: Amit Phulera Date: Sun, 12 Jun 2022 08:38:40 +0530 Subject: [PATCH 7/7] update: changelog --- Changelog | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Changelog b/Changelog index 87da0615..d31e5ada 100644 --- a/Changelog +++ b/Changelog @@ -4,6 +4,16 @@ Change history ================ +.. _version-2.4.0: + +2.4.0 +===== +:release-date: NA +:release-by: NA + +- Fix [#315](https://github.com/celery/django-celery-results/issues/315) Save args, kwargs and other extended props only when result_extended config is set to True + + .. _version-2.3.1: 2.3.1