schema_registry/proto: Accept protobuf as an encoded file descriptor #3663
Conversation
Signed-off-by: Ben Pope <ben@vectorized.io>
Signed-off-by: Ben Pope <ben@vectorized.io>
Support decoding a protobuf FileDescriptor as encoded protobuf. Fix redpanda-data#3633 Signed-off-by: Ben Pope <ben@vectorized.io>
|
|
Signed-off-by: Ben Pope <ben@vectorized.io>
Signed-off-by: Ben Pope <ben@vectorized.io>
BenPope
force-pushed
the
schema-regstry-protobuf-file-descriptor
branch
from
12602b1
to
dae8b89
Compare
BenPope
added
area/schema-registry
| """ | ||
| Verify basic serialization client | ||
| """ | ||
| protocols = [SchemaType.AVRO, SchemaType.PROTOBUF] |
There was a problem hiding this comment.
This could be a good candidate for the @matrix decorator in ducktape that way you don't need a loop over 2 values.
I don't know if @matrix would work with Enums though.
There was a problem hiding this comment.
Nice idea. It's a bit of a trade-off with execution time. Happy to give it a go.
There was a problem hiding this comment.
This could be a good candidate for the @matrix decorator in ducktape that way you don't need a loop over 2 values.
yeh
Nice idea. It's a bit of a trade-off with execution time. Happy to give it a go.
another benefit of using @matrix is that the test framework treats the points parameter space as separate tests. so if one particular point fails ducktape can report the test configuration rather than forcing dev to go look through logs to see which configuration failed.
| from_dict=lambda d, _: AvroPayload(d['val'])), | ||
| SchemaType.PROTOBUF: | ||
| ProtobufDeserializer(ProtobufPayloadClass) | ||
| }[self.schema_type] |
There was a problem hiding this comment.
Python is not my strong point, is there a clearer alternative?
There was a problem hiding this comment.
I'm by no means a python expert but you could store the dict as a variable.
I like your way though.
There was a problem hiding this comment.
I think the difference would be that the serializers would be shared for multiple calls, although it could easily be made a dict of factories.
There was a problem hiding this comment.
https://www.python.org/dev/peps/pep-0636/ in Python 3.10.
There was a problem hiding this comment.
https://www.python.org/dev/peps/pep-0636/ in Python 3.10.
oh that's nice. looks like 3.10 might have made it into fedora 35 so it's getting out there.
|
It doesn't look like the CI failure is related to this PR |
| co_return co_await build_file_with_refs(dp, store, std::move(schema)); | ||
| co_return co_await build_file_with_refs(dp, store, schema); | ||
| } catch (const exception& e) { | ||
| vlog(plog.warn, "Failed to decode schema: {}", e.what()); |
| from_dict=lambda d, _: AvroPayload(d['val'])), | ||
| SchemaType.PROTOBUF: | ||
| ProtobufDeserializer(ProtobufPayloadClass) | ||
| }[self.schema_type] |
There was a problem hiding this comment.
https://www.python.org/dev/peps/pep-0636/ in Python 3.10.
oh that's nice. looks like 3.10 might have made it into fedora 35 so it's getting out there.
| """ | ||
| Verify basic serialization client | ||
| """ | ||
| protocols = [SchemaType.AVRO, SchemaType.PROTOBUF] |
There was a problem hiding this comment.
This could be a good candidate for the @matrix decorator in ducktape that way you don't need a loop over 2 values.
yeh
Nice idea. It's a bit of a trade-off with execution time. Happy to give it a go.
another benefit of using @matrix is that the test framework treats the points parameter space as separate tests. so if one particular point fails ducktape can report the test configuration rather than forcing dev to go look through logs to see which configuration failed.
Cover letter
Schema registry supports the
.protoversion of protobuf, but the serializers send them as encoded protobuf descriptors.This PR accepts the encoded form of a protobuf definition.
Fix #3633
Release notes
Improvements