You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by "Matthias J. Sax" <ma...@confluent.io> on 2017/06/08 00:24:40 UTC

Re: [DISCUSS] KIP-132: Augment KStream.print to allow extra parameters in the printed string

Hi,

as it turns out, we got two KIPs for the same Jira. KIP-160 duplicates
KIP-132.

@Marc: as you did start the first KIP-132 but did not carry it on from
some point on, I am wondering if you are still interested to work on it?

Sorry for this mix up. Hope we can resolve this properly...



-Matthias



On 4/6/17 8:46 PM, Matthias J. Sax wrote:
> Hi Marc,
> 
> any update on this KIP?
> 
> 
> -Matthias
> 
> 
> On 3/20/17 3:02 PM, Eno Thereska wrote:
>> 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 <ma...@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 <ma...@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 <ma...@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 <matthias@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
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
> 


Re: [DISCUSS] KIP-132: Augment KStream.print to allow extra parameters in the printed string

Posted by "Matthias J. Sax" <ma...@confluent.io>.
As there was no response from Marc, and we got KIP-160 as duplicate, I
am "closing" this KIP without a vote. If there are any objections please
let me know.


-Matthias

On 6/7/17 5:24 PM, Matthias J. Sax wrote:
> Hi,
> 
> as it turns out, we got two KIPs for the same Jira. KIP-160 duplicates
> KIP-132.
> 
> @Marc: as you did start the first KIP-132 but did not carry it on from
> some point on, I am wondering if you are still interested to work on it?
> 
> Sorry for this mix up. Hope we can resolve this properly...
> 
> 
> 
> -Matthias
> 
> 
> 
> On 4/6/17 8:46 PM, Matthias J. Sax wrote:
>> Hi Marc,
>>
>> any update on this KIP?
>>
>>
>> -Matthias
>>
>>
>> On 3/20/17 3:02 PM, Eno Thereska wrote:
>>> 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 <ma...@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 <ma...@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 <ma...@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 <matthias@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
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>