search

matrix-org / python-canonicaljson

Canonical JSON
Apache License 2.0
31 stars 15 forks source link

Decouple frozendict support from the library #59

Closed DMRobertson closed 1 year ago

DMRobertson commented 1 year ago

Commitwise reviewable.

Effectively fixes #58, by allowing consumers (Synapse) to register a preserialisation callback for immutabledict.

Breaking change; will need a semver major bump. Consumers can add back support for frozendict by registering a preserialisation callback.

Synapse 1.62 started requiring semver-bounds on canonicaljson in https://github.com/matrix-org/synapse/pull/13082, and I couldn't see anything else which imports canonicaljson on my machine. So only brand-new PyPI installations of Synapse 1.61 or earlier should be broken by this change. (Indeed, the point of that PR was exactly to allow us to make this kind of change.)

DMRobertson commented 1 year ago

Thanks. I'll hold off from merging until I've tested this with Synapse.

DMRobertson commented 1 year ago

I tested Synapse against this in https://github.com/matrix-org/synapse/pull/15113/commits/266370cabc5be94be868a3a976d2bf1b3e2bf6ae and https://github.com/matrix-org/synapse/pull/15113/commits/03e63ab95f5ee77e467f867f2c5a74df0b72bbf7. This passed trial and sytest, see https://github.com/matrix-org/synapse/actions/runs/4419129723/jobs/7747311770 (The other failures are due to me using a git+https:// dependency rather than a pypi one.) So I'm doubly encouraged that this change works.