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

Reply via email to