Fix type error in S3 backend caused by key / task ID by MrSaints · Pull Request #5721 · celery/celery

MrSaints · 2019-09-10T16:10:43Z

The S3 backend for Celery was introduced in cd6c744. The key received by def get(self, key), and def set(self, key, value) however, is of type bytes causing one of the following exceptions in Python 3.7:

TypeError                                 Traceback (most recent call last)
<ipython-input-14-d851513b5988> in <module>
----> 1 result.ready()

/usr/local/lib/python3.7/site-packages/celery/result.py in ready(self)
    311         for retry then :const:`False` is returned.
    312         """
--> 313         return self.state in self.backend.READY_STATES
    314 
    315     def successful(self):

/usr/local/lib/python3.7/site-packages/celery/result.py in state(self)
    471                 then contains the tasks return value.
    472         """
--> 473         return self._get_task_meta()['status']
    474     status = state  # XXX compat
    475 

/usr/local/lib/python3.7/site-packages/celery/result.py in _get_task_meta(self)
    410     def _get_task_meta(self):
    411         if self._cache is None:
--> 412             return self._maybe_set_cache(self.backend.get_task_meta(self.id))
    413         return self._cache
    414 

/usr/local/lib/python3.7/site-packages/celery/backends/base.py in get_task_meta(self, task_id, cache)
    384                 pass
    385 
--> 386         meta = self._get_task_meta_for(task_id)
    387         if cache and meta.get('status') == states.SUCCESS:
    388             self._cache[task_id] = meta

/usr/local/lib/python3.7/site-packages/celery/backends/base.py in _get_task_meta_for(self, task_id)
    725     def _get_task_meta_for(self, task_id):
    726         """Get task meta-data for a task by id."""
--> 727         meta = self.get(self.get_key_for_task(task_id))
    728         if not meta:
    729             return {'status': states.PENDING, 'result': None}

/usr/local/lib/python3.7/site-packages/celery/backends/s3.py in get(self, key)
     58 
     59     def get(self, key):
---> 60         s3_object = self._get_s3_object(key)
     61         try:
     62             s3_object.load()

/usr/local/lib/python3.7/site-packages/celery/backends/s3.py in _get_s3_object(self, key)
     54 
     55     def _get_s3_object(self, key):
---> 56         key_bucket_path = self.base_path + key if self.base_path else key
     57         return self._s3_resource.Object(self.bucket_name, key_bucket_path)
     58 

TypeError: can only concatenate str (not "bytes") to str

OR...

---------------------------------------------------------------------------
ParamValidationError                      Traceback (most recent call last)
<ipython-input-17-d851513b5988> in <module>
----> 1 result.ready()

/usr/local/lib/python3.7/site-packages/celery/result.py in ready(self)
    311         for retry then :const:`False` is returned.
    312         """
--> 313         return self.state in self.backend.READY_STATES
    314 
    315     def successful(self):

/usr/local/lib/python3.7/site-packages/celery/result.py in state(self)
    471                 then contains the tasks return value.
    472         """
--> 473         return self._get_task_meta()['status']
    474     status = state  # XXX compat
    475 

/usr/local/lib/python3.7/site-packages/celery/result.py in _get_task_meta(self)
    410     def _get_task_meta(self):
    411         if self._cache is None:
--> 412             return self._maybe_set_cache(self.backend.get_task_meta(self.id))
    413         return self._cache
    414 

/usr/local/lib/python3.7/site-packages/celery/backends/base.py in get_task_meta(self, task_id, cache)
    384                 pass
    385 
--> 386         meta = self._get_task_meta_for(task_id)
    387         if cache and meta.get('status') == states.SUCCESS:
    388             self._cache[task_id] = meta

/usr/local/lib/python3.7/site-packages/celery/backends/base.py in _get_task_meta_for(self, task_id)
    725     def _get_task_meta_for(self, task_id):
    726         """Get task meta-data for a task by id."""
--> 727         meta = self.get(self.get_key_for_task(task_id))
    728         if not meta:
    729             return {'status': states.PENDING, 'result': None}

/usr/local/lib/python3.7/site-packages/celery/backends/s3.py in get(self, key)
     60         s3_object = self._get_s3_object(key)
     61         try:
---> 62             s3_object.load()
     63             return s3_object.get()['Body'].read().decode('utf-8')
     64         except botocore.exceptions.ClientError as error:

/usr/local/lib/python3.7/site-packages/boto3/resources/factory.py in do_action(self, *args, **kwargs)
    503             # instance via ``self``.
    504             def do_action(self, *args, **kwargs):
--> 505                 response = action(self, *args, **kwargs)
    506                 self.meta.data = response
    507             # Create the docstring for the load/reload mehtods.

/usr/local/lib/python3.7/site-packages/boto3/resources/action.py in __call__(self, parent, *args, **kwargs)
     81                     operation_name, params)
     82 
---> 83         response = getattr(parent.meta.client, operation_name)(**params)
     84 
     85         logger.debug('Response: %r', response)

/usr/local/lib/python3.7/site-packages/botocore/client.py in _api_call(self, *args, **kwargs)
    355                     "%s() only accepts keyword arguments." % py_operation_name)
    356             # The "self" in this scope is referring to the BaseClient.
--> 357             return self._make_api_call(operation_name, kwargs)
    358 
    359         _api_call.__name__ = str(py_operation_name)

/usr/local/lib/python3.7/site-packages/botocore/client.py in _make_api_call(self, operation_name, api_params)
    632         }
    633         request_dict = self._convert_to_request_dict(
--> 634             api_params, operation_model, context=request_context)
    635 
    636         service_id = self._service_model.service_id.hyphenize()

/usr/local/lib/python3.7/site-packages/botocore/client.py in _convert_to_request_dict(self, api_params, operation_model, context)
    680             api_params, operation_model, context)
    681         request_dict = self._serializer.serialize_to_request(
--> 682             api_params, operation_model)
    683         if not self._client_config.inject_host_prefix:
    684             request_dict.pop('host_prefix', None)

/usr/local/lib/python3.7/site-packages/botocore/validate.py in serialize_to_request(self, parameters, operation_model)
    295                                                     operation_model.input_shape)
    296             if report.has_errors():
--> 297                 raise ParamValidationError(report=report.generate_report())
    298         return self._serializer.serialize_to_request(parameters,
    299                                                      operation_model)

ParamValidationError: Parameter validation failed:
Invalid type for parameter Key, value: b'celery-task-meta-fa640a6d-e046-46a5-975a-05c4320bab75', type: <class 'bytes'>, valid types: <class 'str'>

This PR introduces test parameters that would have caught the "bug", and a fix that addresses the failing test.

This is to test setting, and retrieving a key as bytes in addition to string. The current S3 backend does not support the former despite the internals of Celery mostly representing the key as bytes.

… only concatenate str (not "bytes") to str` The task ID is handled internally as "bytes", but it is being treated as a string. This commit follows the `couchdb` backend implementation by converting the key from "bytes" to "string" using kombu's `bytes_to_str`.

auvipy · 2019-09-10T16:49:08Z

any related issue?

MrSaints · 2019-09-10T17:10:22Z

@auvipy Not that I am aware of.

* Update S3 backend test to include `key` as bytes in addition to string This is to test setting, and retrieving a key as bytes in addition to string. The current S3 backend does not support the former despite the internals of Celery mostly representing the key as bytes. * Fix type error in S3 backend: `Invalid type for parameter Key` / `can only concatenate str (not "bytes") to str` The task ID is handled internally as "bytes", but it is being treated as a string. This commit follows the `couchdb` backend implementation by converting the key from "bytes" to "string" using kombu's `bytes_to_str`.

MrSaints added 2 commits September 10, 2019 16:16

Update S3 backend test to include key as bytes in addition to string

9ecfe95

This is to test setting, and retrieving a key as bytes in addition to string. The current S3 backend does not support the former despite the internals of Celery mostly representing the key as bytes.

auvipy approved these changes Sep 10, 2019

View reviewed changes

auvipy merged commit f095f11 into celery:master Sep 10, 2019

auvipy added the PR Type: Bugfix label Sep 10, 2019

auvipy added this to the 4.4.0 milestone Sep 10, 2019

auvipy added the S3 Result backend label Sep 10, 2019

razorx89 mentioned this pull request Dec 10, 2020

Fix type error in S3 backend #6537

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix type error in S3 backend caused by key / task ID#5721

Fix type error in S3 backend caused by key / task ID#5721
auvipy merged 2 commits intocelery:masterfrom
MrSaints:fix-s3-backend-typeerror

MrSaints commented Sep 10, 2019

Uh oh!

auvipy commented Sep 10, 2019

Uh oh!

MrSaints commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

MrSaints commented Sep 10, 2019

Uh oh!

auvipy commented Sep 10, 2019

Uh oh!

MrSaints commented Sep 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants