Open Stack

TLDR: I've abandoned the revert.

After looking at Gibi's investigation further I agree that rabbit was 
actually using the jsonutils version of dumps, so making the fake driver 
use it is consistent. Apologies for the confusion.

-Ben

On 10/1/19 3:35 PM, Ken Giusti wrote:
> Sorry I'm late to the party....
> 
> At the risk of stating the obvious I wouldn't put much faith in the fact 
> that the Kafka and Amqp1 drivers use jsonutils.   The use of jsonutils 
> in these drivers is simply a cut-n-paste from the way old qpidd 
> driver.    Why jsonutils was used there... I dunno.
> 
> IMHO the RabbitMQ driver is the authoritative source for correct driver 
> implementation - the Fake driver (and the others) should use the same 
> serialization as the rabbitmq driver if possible.
> 
> -K
> 
> On Tue, Oct 1, 2019 at 4:30 AM Balázs Gibizer <balazs.gibizer at est.tech> 
> wrote:
> 
> 
> 
>     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 <mailto: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@11
> 
>     Cheers,
>     gibi
> 
>      >
>      > Cheers,
>      > gibi
>      >
>      >
>      > [1]
>      >
>     https://github.com/openstack/oslo.messaging/blob/7734ac1376a1a9285c8245a91cf43599358bfa9d/oslo_messaging/_drivers/amqpdriver.py#L599
>      >
>      >>
>      >>  Thanks.
>      >>
>      >>  -Ben
>      >>
>      >
>      >
> 
> 
> 
> 
> -- 
> Ken Giusti  (kgiusti at gmail.com <mailto:kgiusti at gmail.com>)