Support binary serialization of ProtoBuf schemas #755
Conversation
tvainika
force-pushed
the
protobuf-binary-format
branch
2 times, most recently
from
369698e to
8d6d7d1
Compare
tvainika
force-pushed
the
protobuf-binary-format
branch
from
8d6d7d1 to
2f40f12
Compare
There was a problem hiding this comment.
Some missing handling of elements in the serialization:
ExtendElementnot supported.ExtensionsElementnot supported, this is mostlyproto2but can be used inproto3for declaring custom options.GroupElementnot supported, forproto2.ReservedElementnot supported.
karapace/protobuf/serialization.py
Outdated
| d.nested_type.append(_serialize_type(nt)) | ||
| elif isinstance(nt, EnumElement): | ||
| d.enum_type.append(_serialize_enumtype(nt)) | ||
| if isinstance(t, MessageElement): |
There was a problem hiding this comment.
What would the else be here?
There was a problem hiding this comment.
Added else branch throwing an exception
There was a problem hiding this comment.
Ok, understood your point wrongly first. Now fixed by renaming _serialize_type -> _serialize_msgtype and only takes MessageElement
There was a problem hiding this comment.
Can we take other open repos containing the serialized format to do kinda like an integration test? I'm wondering if we can expand the testing coverage in an un-expensive way to automatically catch missing part. For the protobuf @jjaakola-aiven have parsed in an automatic way a huge amount of protobuf file. In that case maybe we can do the same by comparing an existing and working serializer with your
tvainika
force-pushed
the
protobuf-binary-format
branch
from
2f40f12 to
9ffa7e9
Compare
|
Added support for Somehow reusing open repository of different schemas would be nice, but is there any repository that would have also base64 encoded binary schemas in parallel readymade? If not, then testing against such requires somewhat manual effort to verify multiple things for each schema. |
tvainika
force-pushed
the
protobuf-binary-format
branch
from
9ffa7e9 to
0ac4152
Compare
There was a problem hiding this comment.
To me looks fine, waiting @jjaakola-aiven feedback before merging
Add support for base64 encoded binary encodings of ProtoBuf schemas when registering new schemas and with format=serialized query parameter when fetching the schemas. Fixes #742
tvainika
force-pushed
the
protobuf-binary-format
branch
from
0ac4152 to
68ede88
Compare
About this change - What it does
Add support for base64 encoded binary encodings of ProtoBuf schemas when registering new schemas and with format=serialized query parameter when fetching the schemas. Fixes #742 #398
Why this way
Current implementation uses protobuf python package for binary serialization. On top of that it needs both ways converting between internal protobuf schema presentation and protobuf library definition of types.