[oslo][nova] Revert of oslo.messaging JSON serialization change
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.
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.
> On Tue, Oct 1, 2019 at 4:30 AM BalÃ¡zs Gibizer <balazs.gibizer at est.tech>
> 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
> >>Â 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
> >>Â 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
> >>Â 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
> > 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
> > the AMQPDriverBase ._send() is called that therefore jsonutils is
> > 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
> > 
> >>Â Thanks.
> >>Â -Ben
> Ken GiustiÂ (kgiusti at gmail.com <mailto:kgiusti at gmail.com>)