bananaaggle commented on pull request #10839: URL: https://github.com/apache/druid/pull/10839#issuecomment-791102703
> lgtm, thanks 👍 > > @clintropolis Hi, thanks for your review. I've refine document for this feature. What else should I do before merging? > > I think this is g2g after CI. > > If you are interested in doing a follow-up PR or two, the protobuf extension is one of the few ingestion formats that has not been ported to the newer [`InputFormat`](https://github.com/apache/druid/blob/master/core/src/main/java/org/apache/druid/data/input/InputFormat.java) interface which has replaced parsers. There are a handful of implementations that could be used as reference ([JSON](https://github.com/apache/druid/blob/master/core/src/main/java/org/apache/druid/data/input/impl/JsonInputFormat.java), [Avro](https://github.com/apache/druid/blob/master/extensions-core/avro-extensions/src/main/java/org/apache/druid/data/input/avro/AvroOCFInputFormat.java) though it is missing an `InputFormat` for streaming support and using schema registry itself, and many others). I think this should be relatively straight-forward with the protobuf extension since most of the work is being done in the decoder implementations, which could also be re-used by the `InputFormat` implementation. > > Another thing, after #10929 goes in it would probably be possible to add an integration test for the functionality added in this PR. A fair warning, our integration tests are a bit rough around the edges to work with, but I think most of the parts would be there (I guess the protobuf jar we can't include would need [added to the Dockerfile similar to mysql](https://github.com/apache/druid/blob/master/integration-tests/docker/Dockerfile#L44)). I'm not sure we actually have _any_ integration tests using protobuf currently. Yeah, I will learn something about InputFormat interface and implement it with protobuf. As for integration test, I'm not very familiar with this component, but I think I can finish it. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
