Hi Marc, Could you add more information in the motivation of the KIP as to what problems this would solve? I can see how it can be done, but I don't yet grok why it's useful. The KIP should contain more pain points/problems and pose this as a solution. I know it's a small modification, but it's still important to have a good motivation IMO.
Thanks Eno > On 20 Mar 2017, at 18:25, Matthias J. Sax <matth...@confluent.io> wrote: > > Sound reasonable Damian, but I guess, that's more a PR than KIP discussion. > > @Marc, I guess you can start a VOTE thread if there is no further feedback. > > > -Matthias > > On 3/20/17 7:06 AM, Damian Guy wrote: >> 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 >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >> >
