celery / kombu

Messaging library for Python.
http://kombu.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
2.87k stars 928 forks source link

Call out breaking change in 5.3.0 #1821

Open mrcljx opened 11 months ago

mrcljx commented 11 months ago

I spent two hours trying to figure out why Kombu suddenly started decoding datetime objects. After searching the issues, I learned this was a fix of a long-standing regression.

But since 5.2.4 had this issue for 15 months and people will have written code based on that behavior, it would be great if this was called out in the changelog, maybe even offering a workaround:

kombu.serialization.unregister("json")
kombu.serialization.register(
    "json",
    json.dumps,
    json.loads,
    content_type="application/json",
    content_encoding="utf-8",
)
auvipy commented 11 months ago

We generally accept breaking changes in 5.2.0 and 5.3.0 versions. the changes were documented but due to some problem in CI release note script it was overridden twice. would you mind share some workaround so that I can re add them to the 5.3.x upgrade consideration?

mrcljx commented 11 months ago

I understand the need for this breaking change. It just came as a surprise when upgrading Celery, so a callout was all I needed. 🙂

I'm going forward with this workaround (our test suite passes again):

kombu.serialization.unregister("json")
kombu.serialization.register(
    "json",
    json.dumps, # maybe: orjson
    json.loads,
    content_type="application/json",
    content_encoding="utf-8",
)

Could be that this workaround is flawed for messages in flight - I couldn't really see whether 5.2 would also emit __type__ for some fields that the above decoder wouldn't be able to handle.

Off-topic: I haven't seen the custom JSON coding behavior defined anywhere. Would be great to have a blurb (around __type__ and __value__) in the docs at least. Especially as json is considered portable in contrast to pickle.

auvipy commented 11 months ago

I highly appreciate your inputs here. I actually forgot. Now I will revisit the release notes again to mention these type of un intended breaking changes and work around to consider before upgrade. thanks