From bb4121a465d99d15bd33693833fdc9e94b07f0f8 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 11 Mar 2016 09:00:08 +0000 Subject: [PATCH] Revert "Ensure the json result type is bytes on Python 3" This reverts commit bd81d09c02c5bc8561ad04de91802a5c1917d9e9. I understand that the change was supposed to fix something, but instead it broke all tests on Python 3!? It's wrong to replace blindly json.dumps() with jsonutils.dump_as_bytes(). In oslo messaging, the result is usually used as a value in a dictionary, and then the whole dictionary is passed to a second serializer which also serialize to JSON. Sorry, I don't understand everything, but at least I see that tests passed on py3 before the change, and started to fail with the change. Maybe json(utils).dumps() is misused in some places, but in this case, you should write a change which only fix these specific places, not replace all calls to dumps(). Change-Id: Icd54ee8e3f5c976dfd50b4b62c7f51288649e112 --- oslo_messaging/_drivers/common.py | 6 +++--- oslo_messaging/_drivers/impl_kafka.py | 2 +- oslo_messaging/_drivers/protocols/amqp/driver.py | 4 ++-- oslo_messaging/notify/_impl_log.py | 1 - oslo_messaging/tests/drivers/test_impl_kafka.py | 2 +- oslo_messaging/tests/drivers/test_impl_rabbit.py | 2 +- oslo_messaging/tests/test_exception_serialization.py | 2 +- 7 files changed, 9 insertions(+), 10 deletions(-) diff --git a/oslo_messaging/_drivers/common.py b/oslo_messaging/_drivers/common.py index bad575832..7b446d74d 100644 --- a/oslo_messaging/_drivers/common.py +++ b/oslo_messaging/_drivers/common.py @@ -197,13 +197,13 @@ def serialize_remote_exception(failure_info, log_failure=True): 'kwargs': kwargs } - json_data = jsonutils.dump_as_bytes(data) + json_data = jsonutils.dumps(data) return json_data def deserialize_remote_exception(data, allowed_remote_exmods): - failure = jsonutils.loads(data) + failure = jsonutils.loads(six.text_type(data)) trace = failure.get('tb', []) message = failure.get('message', "") + "\n" + "\n".join(trace) @@ -284,7 +284,7 @@ def serialize_msg(raw_msg): # NOTE(russellb) See the docstring for _RPC_ENVELOPE_VERSION for more # information about this format. msg = {_VERSION_KEY: _RPC_ENVELOPE_VERSION, - _MESSAGE_KEY: jsonutils.dump_as_bytes(raw_msg)} + _MESSAGE_KEY: jsonutils.dumps(raw_msg)} return msg diff --git a/oslo_messaging/_drivers/impl_kafka.py b/oslo_messaging/_drivers/impl_kafka.py index 79511fef3..11b72bb28 100644 --- a/oslo_messaging/_drivers/impl_kafka.py +++ b/oslo_messaging/_drivers/impl_kafka.py @@ -126,7 +126,7 @@ class Connection(object): def _send_and_retry(self, message, topic, retry): current_retry = 0 if not isinstance(message, str): - message = jsonutils.dump_as_bytes(message) + message = jsonutils.dumps(message) while message is not None: try: self._send(message, topic) diff --git a/oslo_messaging/_drivers/protocols/amqp/driver.py b/oslo_messaging/_drivers/protocols/amqp/driver.py index 68efdcb3c..04feb2de1 100644 --- a/oslo_messaging/_drivers/protocols/amqp/driver.py +++ b/oslo_messaging/_drivers/protocols/amqp/driver.py @@ -56,7 +56,7 @@ def marshal_response(reply=None, failure=None): data = {"failure": failure} else: data = {"response": reply} - msg.body = jsonutils.dump_as_bytes(data) + msg.body = jsonutils.dumps(data) return msg @@ -80,7 +80,7 @@ def marshal_request(request, context, envelope): "request": request, "context": context } - msg.body = jsonutils.dump_as_bytes(data) + msg.body = jsonutils.dumps(data) return msg diff --git a/oslo_messaging/notify/_impl_log.py b/oslo_messaging/notify/_impl_log.py index ceca6d451..7322f07b6 100644 --- a/oslo_messaging/notify/_impl_log.py +++ b/oslo_messaging/notify/_impl_log.py @@ -40,7 +40,6 @@ class LogDriver(notifier.Driver): message['event_type'])) method = getattr(logger, priority.lower(), None) if method: - # NOTE: The logger needs json formatted string instead of bytes method(jsonutils.dumps(strutils.mask_dict_password(message))) else: warnings.warn('Unable to log message as notify cannot find a ' diff --git a/oslo_messaging/tests/drivers/test_impl_kafka.py b/oslo_messaging/tests/drivers/test_impl_kafka.py index 5843d92ce..7f1d5e377 100644 --- a/oslo_messaging/tests/drivers/test_impl_kafka.py +++ b/oslo_messaging/tests/drivers/test_impl_kafka.py @@ -145,7 +145,7 @@ class TestKafkaConnection(test_utils.BaseTestCase): conn.consumer = mock.MagicMock() conn.consumer.fetch_messages = mock.MagicMock( - return_value=iter([jsonutils.dump_as_bytes(fake_message)])) + return_value=iter([jsonutils.dumps(fake_message)])) self.assertEqual(fake_message, jsonutils.loads(conn.consume()[0])) self.assertEqual(1, len(conn.consumer.fetch_messages.mock_calls)) diff --git a/oslo_messaging/tests/drivers/test_impl_rabbit.py b/oslo_messaging/tests/drivers/test_impl_rabbit.py index b1040f26f..bb449606d 100644 --- a/oslo_messaging/tests/drivers/test_impl_rabbit.py +++ b/oslo_messaging/tests/drivers/test_impl_rabbit.py @@ -919,7 +919,7 @@ class TestReplyWireFormat(test_utils.BaseTestCase): '_reply_q': 'reply_' + uuid.uuid4().hex, }) - msg['oslo.message'] = jsonutils.dump_as_bytes(msg['oslo.message']) + msg['oslo.message'] = jsonutils.dumps(msg['oslo.message']) producer.publish(msg) diff --git a/oslo_messaging/tests/test_exception_serialization.py b/oslo_messaging/tests/test_exception_serialization.py index 0b2998c00..c1079c0a4 100644 --- a/oslo_messaging/tests/test_exception_serialization.py +++ b/oslo_messaging/tests/test_exception_serialization.py @@ -293,7 +293,7 @@ class DeserializeRemoteExceptionTestCase(test_utils.BaseTestCase): 'kwargs': self.kwargs, } - serialized = jsonutils.dump_as_bytes(failure) + serialized = jsonutils.dumps(failure) ex = exceptions.deserialize_remote_exception(serialized, self.allowed)