Regression in 2.2.0: `yaml.dump` can no longer save capnp messages with `Data` type fields

[OfekShilon]

In a nutshell: when schema files define fields with type Data, pycapnp <= 2.1.0 compiled this schema into python attributes of type bytes. pycapnp 2.2.0 compiles them to python attributes of type memoryview, which are subject to the limitation that they cannot be pickled.

So attempts to pickle - even indirectly, as yaml.dump does - fail, and existing code breaks.

A small repro is available in https://github.com/OfekShilon/capnp.

[haata]

@BrianXu0623 any thoughts on this?

Worst case we may need to revert #380

[OfekShilon]

Sorry, the repo in the link was accidentally private. Now public.

[BrianXu0623]

Hi — thanks for the repro and for calling this out.

Background

Cap’n Proto’s C++ layer exposes Data fields as pointers into the message’s memory. Returning bytes forces an immediate copy of that buffer. To avoid that expensive copy (which is a real performance and memory bottleneck for large messages), I made it return a memoryview instead of bytes. (I didn't find other better built-in Python types than memoryview for zero-copy access to Cap’n Proto Data fields, though it cannot be directly serialized.）

Temporary Solution (add a YAML layer)

Rather than reverting the zero-copy behavior, one temporary approach is to add a small YAML representer so yaml.dump() knows how to serialize memoryview. This kind of explicit encoding / copy is needed in Python when serializing to text formats (YAML/JSON), because memoryview cannot be pickled, but in C++ this is unnecessary..

import base64, yaml

yaml.add_representer(memoryview,
    lambda d, m: d.represent_scalar('tag:yaml.org,2002:binary', base64.b64encode(bytes(m)).decode('ascii')))

yaml.SafeDumper.add_representer(memoryview,
    lambda d, m: d.represent_scalar('tag:yaml.org,2002:binary', base64.b64encode(bytes(m)).decode('ascii')))

@haata @OfekShilon This may not be the ideal solution, I can also submit a patch to modify the message.to_dict() method directly instead. (Please see the post below)

[BrianXu0623]

Created a pull request for this issue.

This can resolve the issue without needing change to user's code. I think this should be the right way for to_dict serialisation, instead of letting user add the YAML layer. Sorry for introducing this problem.

Please help review when you have time ~ @haata
Thanks

[OfekShilon]

@BrianXu0623 Thanks for this! This PR will definitely fix our particular use case, going through to_dict. Hopefully there isn't (or not much) pycapnp serialization code that doesn't go through it.

[haata]

I'll try to push a release later this week if I have some time.

[haata]

Thanks @BrianXu0623 !

[OfekShilon]

Since a fix is merged, shouldn't this issue be closed? (whichever way you prefer)

[OfekShilon]

Any estimate on release of a version with the fix?

[haata]

Sorry for the delay, https://pypi.org/project/pycapnp/2.2.1/ is now available.

[theunkn0wn1]

The fix released in 2.2.1 is only partial fix , and the breaking behavior persists in other places.
In any message that deserializes from capnp format Data now becomes a messageview object.

This breaks any downstream code (including my projects!) that was written to the understanding Data -> bytes and extends beyond YAML interpretation.

To keep this change, the PR should have resulted in a major version bump to adhere to SEMVER.

[BrianXu0623]

The fix released in 2.2.1 is only partial fix , and the breaking behavior persists in other places. In any message that deserializes from capnp format Data now becomes a messageview object.

This breaks any downstream code (including my projects!) that was written to the understanding Data -> bytes and extends beyond YAML interpretation.

To keep this change, the PR should have resulted in a major version bump to adhere to SEMVER.

Thanks for the suggestion, and sorry for any inconvenience being introduced by this change. I've created a new PR for resolving this issue, please take a look when you are available.

CC @haata @theunkn0wn1

#390