clintropolis edited a comment on pull request #10839:
URL: https://github.com/apache/druid/pull/10839#issuecomment-790505170


   >@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 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.


----------------------------------------------------------------
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]

Reply via email to