+1 (binding) Thanks for the KIP.
On Tue, May 19, 2020 at 2:57 PM Mickael Maison <mickael.mai...@gmail.com> wrote: > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > >
