codehaus


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[oslo][nova] Revert of oslo.messaging JSON serialization change



On Mon, Sep 30, 2019 at 5:35 PM, Balázs Gibizer 
<balazs.gibizer at est.tech> wrote:
> 
> 
> On Mon, Sep 30, 2019 at 4:45 PM, Ben Nemec <openstack at nemebean.com>
> wrote:
>>  Hi,
>> 
>>  I've just proposed https://review.opendev.org/#/c/685724/ which
>>  reverts a change that recently went in to make the fake driver in
>>  oslo.messaging use jsonutils for message serialization instead of
>>  json.dumps.
>> 
>>  As explained in the commit message on the revert, this is 
>> problematic
>>  because the rabbit driver uses kombu's default serialization method,
>>  which is json.dumps. By changing the fake driver to use jsonutils
>>  we've made it more lenient than the most used real driver which 
>> opens
>>  us up to merging broken changes in consumers of oslo.messaging.
>> 
>>  We did have some discussion of whether we should try to override the
>>  kombu default and tell it to use jsonutils too, as a number of other
>>  drivers do. The concern with this was that the jsonutils handler for
>>  things like datetime objects is not tz-aware, which means if you 
>> send
>>  a datetime object over RPC and don't explicitly handle it you could
>>  lose important information.
>> 
>>  I'm open to being persuaded otherwise, but at the moment I'm leaning
>>  toward less magic happening at the RPC layer and requiring projects
>>  to explicitly handle types that aren't serializable by the standard
>>  library json module. If you have a different preference, please 
>> share
>>  it here.
> 
> Hi,
> 
> I might me totally wrong here and please help me understand how the
> RabbitDriver works. What I did when I created the original patch that 
> I
> looked at each drivers how they handle sending messages. The
> oslo_messaging._drivers.base.BaseDriver defines the interface with a
> send() message. The oslo_messaging._drivers.amqpdriver.AMQPDriverBase
> implements the BaseDriver interface's send() method to call _send().
> Then _send() calls rpc_commom.serialize_msg which then calls
> jsonutils.dumps.
> 
> The oslo_messaging._drivers.impl_rabbit.RabbitDriver driver inherits
> from AMQPDriverBase and does not override send() or _send() so I think
> the AMQPDriverBase ._send() is called that therefore jsonutils is used
> during sending a message with RabbitDriver.

I did some tracing in devstack to prove my point. See the result in 
https://review.opendev.org/#/c/685724/1//COMMIT_MSG at 11

Cheers,
gibi

> 
> Cheers,
> gibi
> 
> 
> [1]
> https://github.com/openstack/oslo.messaging/blob/7734ac1376a1a9285c8245a91cf43599358bfa9d/oslo_messaging/_drivers/amqpdriver.py#L599
> 
>> 
>>  Thanks.
>> 
>>  -Ben
>> 
> 
>