Thanks Konstantine for the feedback (and vote)! 1) I've added example commands using the new formatters
2) I updated the Compatiblity section to be more explicit about the need for recompilation 3) Good point, updated On Tue, May 19, 2020 at 3:18 AM Konstantine Karantasis <konstant...@confluent.io> wrote: > > Thanks Michael. > I think it's useful to enable specialized message formatters by adding this > interface to the public API. > > You have my vote: +1 (binding) > > Just a few optional comments below: > > 1. Would you mind adding the equivalent command line example in the places > where you have an example output? > > Something equivalent to > ./bin/kafka-console-consumer.sh --bootstrap-server localhost:9092 --topic > __consumer_offsets --from-beginning --formatter > "kafka.coordinator.group.GroupMetadataManager\$GroupMetadataMessageFormatter" > > but with the equivalent formatter classes and expected topic names. > > 2. I have to note that breaking old formatters by requiring recompilation > could be avoided if we didn't change kafka.common.MessageFormatter to > extend the new org.apache.kafka.common.MessageFormatter. We could maintain > both, while the old one would remain deprecated and we could attempt to > instantiate one or the other type when reading the config and use either of > the two different types in the few places in ConsoleConsumer that a > formatter is used. But I admit that for this use case, it's not worth > maintaining both types. The interface wasn't public before anyways. > > Given that, my small request would be to rephrase in the compatibility > section to say something like: > 'Existing MessageFormatters implementations will require no changes beyond > recompilation.' or similar. Because, to be precise, existing formatters > _won't_ work if they are given as a parameter to a 2.6 console consumer, > without recompilation as you mention. > > 3. Finally, a minor comment on skipping the use of the `public` specifier > in the interface because it's redundant. > > Best regards, > Konstantine > > On Mon, May 18, 2020 at 3:26 PM Maulin Vasavada <maulin.vasav...@gmail.com> > wrote: > > > +1 (non-binding) > > > > On Mon, May 18, 2020 at 8:49 AM Mickael Maison <mickael.mai...@gmail.com> > > wrote: > > > > > Bumping this thread as KIP freeze is approaching. > > > > > > It's a pretty small change and I have a PR ready: > > > https://github.com/apache/kafka/pull/8604 > > > > > > Thanks > > > > > > On Mon, May 4, 2020 at 5:26 PM Ryanne Dolan <ryannedo...@gmail.com> > > wrote: > > > > > > > > +1, non-binding > > > > > > > > On Mon, May 4, 2020, 9:24 AM Christopher Egerton <chr...@confluent.io> > > > > wrote: > > > > > > > > > +1 (non-binding) > > > > > > > > > > On Mon, May 4, 2020 at 5:02 AM Edoardo Comar <eco...@uk.ibm.com> > > > wrote: > > > > > > > > > > > +1 (non-binding) > > > > > > Thanks Mickael > > > > > > > > > > > > -------------------------------------------------- > > > > > > > > > > > > Edoardo Comar > > > > > > > > > > > > Event Streams for IBM Cloud > > > > > > IBM UK Ltd, Hursley Park, SO21 2JN > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > From: Mickael Maison <mickael.mai...@gmail.com> > > > > > > To: dev <dev@kafka.apache.org> > > > > > > Date: 04/05/2020 11:45 > > > > > > Subject: [EXTERNAL] [VOTE] KIP-597: MirrorMaker2 internal > > > topics > > > > > > Formatters > > > > > > > > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > I'd like to start a vote on KIP-597: > > > > > > > > > > > > > > > > > > > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D597-253A-2BMirrorMaker2-2Binternal-2Btopics-2BFormatters&d=DwIBaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ&m=r-_T9EFUWNEUGi1GuX7klXNZIH2sJmxGTtySV3lAjoQ&s=iyBSxabuEi1h7ksmzoXgJT8jJoMR0xKYsJy_MpvtCRQ&e= > > > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Unless stated otherwise above: > > > > > > IBM United Kingdom Limited - Registered in England and Wales with > > > number > > > > > > 741598. > > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire > > > PO6 > > > > > 3AU > > > > > > > > > > > > > > > > > > > > > >
