-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Replace "protobuf-dynamic" lib with built-in protobuf methods #18401
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
public abstract class DescriptorBasedProtobufBytesDecoder implements ProtobufBytesDecoder | ||
{ | ||
private Descriptors.Descriptor descriptor; | ||
|
||
/** | ||
* An optional message Protobuf message type in the descriptor. Both short name and fully qualified name are accepted. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from our docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added a few comments to this file (and made the constructor a bit cleaner) while reading through this file to understand what is happening.
...ns/src/test/java/org/apache/druid/data/input/protobuf/FileBasedProtobufBytesDecoderTest.java
Fixed
Show fixed
Hide fixed
...ns/src/test/java/org/apache/druid/data/input/protobuf/FileBasedProtobufBytesDecoderTest.java
Fixed
Show fixed
Hide fixed
...test/java/org/apache/druid/data/input/protobuf/InlineDescriptorProtobufBytesDecoderTest.java
Fixed
Show fixed
Hide fixed
...test/java/org/apache/druid/data/input/protobuf/InlineDescriptorProtobufBytesDecoderTest.java
Fixed
Show fixed
Hide fixed
Thanks Jon.
Description
The inline and file-based protobuf decoders depended on an external library protobuf-dynamic. Initially, I thought the library (which appears to be no longer actively maintained) doesn't support maps at all, but upon further testing, I was unable to confirm this.
However, instead of using a third-party lib, we can achieve the same functionality (building a Protobuf descriptor from a file or Base64-encoded string) using the standard protobuf library. This PR does so.
Additionally, this PR improves the code generation used in unit tests a lot:
This PR has: