Hi Marc, Thanks for the KIP. It mostly looks good to me. The only thing i'd change is using a null argument to use a default mapping. IMO it would be better if the existing print() method delegates to the new one supplying a KeyValueMapper that does the right thing.
Thanks, Damian On Sat, 18 Mar 2017 at 14:25 Marc Juchli <m...@marcjuch.li> wrote: > Thanks! > > I wanted to PING this thread. Not sure what the next steps of the KIP > process are? > > Kind regards, > Marc > > On Wed, Mar 15, 2017 at 9:13 PM Matthias J. Sax <matth...@confluent.io> > wrote: > > > Thanks for updating the KIP. > > > > It's in very good shape IMHO and I support this idea! > > > > > > > > -Matthias > > > > > > On 3/15/17 3:05 AM, Marc Juchli wrote: > > > Dear Matthias, > > > > > > The KIP is updated. I think it now contains all the information on that > > > page. > > > > > > Marc > > > > > > On Mon, Mar 13, 2017 at 9:37 PM Matthias J. Sax <matth...@confluent.io > > > > > wrote: > > > > > >> Marc, > > >> > > >> Thanks for the KIP. > > >> > > >> Can you please update the KIP in a way such that it is self contained. > > >> Right now, you link to all kind of other places making it hard to read > > >> the KIP. > > >> > > >> The KIP should be the "center of truth" -- if there is important > > >> information elsewhere, please c&p it into the KIP. > > >> > > >> > > >> Thanks a lot! > > >> > > >> > > >> -Matthias > > >> > > >> > > >> > > >> On 3/13/17 1:30 PM, Matthias J. Sax wrote: > > >>> Can you please add the KIP to this table: > > >>> > > >>> > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-KIPsunderdiscussion > > >>> > > >>> Thanks, > > >>> > > >>> Matthias > > >>> > > >>> > > >>> On 3/13/17 8:08 AM, Marc Juchli wrote: > > >>>> Dear all, > > >>>> > > >>>> The following describes KIP-132, which I just created. See: > > >>>> > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-132+-+Augment+KStream.print+to+allow+extra+parameters+in+the+printed+string > > >>>> > > >>>> Motivation > > >>>> > > >>>> As for now, KStream#print leads to a predefined output where key and > > >> value are > > >>>> printed with comma separation. > > >>>> KAFKA-4830 <https://issues.apache.org/jira/browse/KAFKA-4830> > > suggests > > >> to > > >>>> extend print in a way that it takes KeyValueMapper as a parameter. > > >>>> This will allow a user to change outputs according to the users > > demand. > > >>>> Public Interfaces > > >>>> > > >>>> The affected interface is KStream, which needs to be extended with > > >> another > > >>>> overloaded version of print: > > >>>> > > >>>> void print(final Serde<K> keySerde, > > >>>> final Serde<V> valSerde, > > >>>> final String streamName, > > >>>> final KeyValueMapper<K, V, String> mapper); > > >>>> > > >>>> Proposed Changes > > >>>> > > >>>> See pull request GH-2669 <https://github.com/apache/kafka/pull/2669 > >. > > >>>> This PR contains a discussion regarding KAFKA-4830 > > >>>> <https://issues.apache.org/jira/browse/KAFKA-4830> as well as > > >> KAFKA-4772 > > >>>> <https://issues.apache.org/jira/browse/KAFKA-4772>. > > >>>> > > >>>> Compatibility, Deprecation, and Migration Plan > > >>>> > > >>>> The extension of print will not introduce compatibility issues – we > > can > > >>>> maintain the current output by keeping the current output format as > a > > >>>> default (if mapper was not set): > > >>>> > > >>>> if(mapper == null) { > > >>>> printStream.println("[" + streamName + "]: " + keyToPrint + " , > " > > >>>> + valueToPrint); > > >>>> } else { > > >>>> printStream.println("[" + streamName + "]: " + > > >>>> mapper.apply(keyToPrint, valueToPrint)); > > >>>> } > > >>>> > > >>>> > > >>>> > > >>>> Kind regards, > > >>>> Marc > > >>>> > > >>> > > >> > > >> > > > > > > > >
