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/02 05:28:17 UTC
Re: KIP-160 - Augment KStream.print() to allow users pass in extra
parameters in the printed string
Thanks for the KIP.
Two comments:
- I think we should include #writeAsText()
- I am not sure if we should use
> "[" + this.streamName + "]: " + mapper.apply(keyToPrint, valueToPrint)
in case a mapper is provided. This still dictates a fixed prefix a user
might not want to have (what contradicts or at least limits the scope of
this new functionality). Considering he current discussion of KIP-159, a
user would be able to access the stream name within the provided mapper
and add it if they wish anyway, and thus, I don't think we should force
this format.
-Matthias
On 5/30/17 1:38 PM, Guozhang Wang wrote:
> Overall +1. One comment about the wiki itself:
>
> Could you replace the general description of "Argument KStream.print() which
> is KStream.print(KeyValueMapper<K, V, String>)" with the actual added
> overloaded functions in the wiki page?
>
>
> Guozhang
>
> On Mon, May 22, 2017 at 12:21 AM, James Chain <ja...@gmail.com>
> wrote:
>
>> Hi All,
>>
>> I want to start this KIP to argument KStream.print().
>> This vote is already started.
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 160+-+Augment+KStream.print%28%29+to+allow+users+pass+in+
>> extra+parameters+in+the+printed+string
>>
>> Thanks,
>>
>> James Chien
>>
>
>
>
Re: KIP-160 - Augment KStream.print() to allow users pass in extra
parameters in the printed string
Posted by Guozhang Wang <wa...@gmail.com>.
I see. LGTM.
On Tue, Jun 6, 2017 at 6:07 AM, Bill Bejeck <bb...@gmail.com> wrote:
> +1 on adding node name to the `RecordContext`
>
> -Bill
>
> On Mon, Jun 5, 2017 at 6:24 PM, Jeyhun Karimov <je...@gmail.com>
> wrote:
>
> > I agree with Matthias's comment. Constructing RecordContext with more
> > metadata seems more feasible for me.
> >
> > Cheers,
> > Jeyun
> >
> > On Mon, Jun 5, 2017 at 7:47 AM Matthias J. Sax <ma...@confluent.io>
> > wrote:
> >
> > > Not with the scope of the current discussion.
> > >
> > > So far, we discuss to add `RecordContext`, but the context object we
> use
> > > could also provide some more metadata. I see no reason why not to
> expose
> > > the node name there. We already expose TaskId vie `ProcessorContext`.
> We
> > > could also add thread name. IMHO, this would be better than dictating
> > > any prefix.
> > >
> > > Thoughts?
> > >
> > >
> > > -Matthias
> > >
> > > On 6/4/17 9:03 PM, Guozhang Wang wrote:
> > > > Matthias,
> > > >
> > > > I think even with KIP-159 users would not be able to access the
> > processor
> > > > node name right?
> > > >
> > > > Guozhang
> > > >
> > > > On Thu, Jun 1, 2017 at 10:28 PM, Matthias J. Sax <
> > matthias@confluent.io>
> > > > wrote:
> > > >
> > > >> Thanks for the KIP.
> > > >>
> > > >> Two comments:
> > > >> - I think we should include #writeAsText()
> > > >> - I am not sure if we should use
> > > >>
> > > >>> "[" + this.streamName + "]: " + mapper.apply(keyToPrint,
> > valueToPrint)
> > > >>
> > > >> in case a mapper is provided. This still dictates a fixed prefix a
> > user
> > > >> might not want to have (what contradicts or at least limits the
> scope
> > of
> > > >> this new functionality). Considering he current discussion of
> > KIP-159, a
> > > >> user would be able to access the stream name within the provided
> > mapper
> > > >> and add it if they wish anyway, and thus, I don't think we should
> > force
> > > >> this format.
> > > >>
> > > >>
> > > >>
> > > >> -Matthias
> > > >>
> > > >>
> > > >>
> > > >> On 5/30/17 1:38 PM, Guozhang Wang wrote:
> > > >>> Overall +1. One comment about the wiki itself:
> > > >>>
> > > >>> Could you replace the general description of "Argument
> > KStream.print()
> > > >> which
> > > >>> is KStream.print(KeyValueMapper<K, V, String>)" with the actual
> > added
> > > >>> overloaded functions in the wiki page?
> > > >>>
> > > >>>
> > > >>> Guozhang
> > > >>>
> > > >>> On Mon, May 22, 2017 at 12:21 AM, James Chain <
> > > james.chain1990@gmail.com
> > > >>>
> > > >>> wrote:
> > > >>>
> > > >>>> Hi All,
> > > >>>>
> > > >>>> I want to start this KIP to argument KStream.print().
> > > >>>> This vote is already started.
> > > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > >>>> 160+-+Augment+KStream.print%28%29+to+allow+users+pass+in+
> > > >>>> extra+parameters+in+the+printed+string
> > > >>>>
> > > >>>> Thanks,
> > > >>>>
> > > >>>> James Chien
> > > >>>>
> > > >>>
> > > >>>
> > > >>>
> > > >>
> > > >>
> > > >
> > > >
> > >
> > > --
> > -Cheers
> >
> > Jeyhun
> >
>
--
-- Guozhang
Re: KIP-160 - Augment KStream.print() to allow users pass in extra
parameters in the printed string
Posted by Bill Bejeck <bb...@gmail.com>.
+1 on adding node name to the `RecordContext`
-Bill
On Mon, Jun 5, 2017 at 6:24 PM, Jeyhun Karimov <je...@gmail.com> wrote:
> I agree with Matthias's comment. Constructing RecordContext with more
> metadata seems more feasible for me.
>
> Cheers,
> Jeyun
>
> On Mon, Jun 5, 2017 at 7:47 AM Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > Not with the scope of the current discussion.
> >
> > So far, we discuss to add `RecordContext`, but the context object we use
> > could also provide some more metadata. I see no reason why not to expose
> > the node name there. We already expose TaskId vie `ProcessorContext`. We
> > could also add thread name. IMHO, this would be better than dictating
> > any prefix.
> >
> > Thoughts?
> >
> >
> > -Matthias
> >
> > On 6/4/17 9:03 PM, Guozhang Wang wrote:
> > > Matthias,
> > >
> > > I think even with KIP-159 users would not be able to access the
> processor
> > > node name right?
> > >
> > > Guozhang
> > >
> > > On Thu, Jun 1, 2017 at 10:28 PM, Matthias J. Sax <
> matthias@confluent.io>
> > > wrote:
> > >
> > >> Thanks for the KIP.
> > >>
> > >> Two comments:
> > >> - I think we should include #writeAsText()
> > >> - I am not sure if we should use
> > >>
> > >>> "[" + this.streamName + "]: " + mapper.apply(keyToPrint,
> valueToPrint)
> > >>
> > >> in case a mapper is provided. This still dictates a fixed prefix a
> user
> > >> might not want to have (what contradicts or at least limits the scope
> of
> > >> this new functionality). Considering he current discussion of
> KIP-159, a
> > >> user would be able to access the stream name within the provided
> mapper
> > >> and add it if they wish anyway, and thus, I don't think we should
> force
> > >> this format.
> > >>
> > >>
> > >>
> > >> -Matthias
> > >>
> > >>
> > >>
> > >> On 5/30/17 1:38 PM, Guozhang Wang wrote:
> > >>> Overall +1. One comment about the wiki itself:
> > >>>
> > >>> Could you replace the general description of "Argument
> KStream.print()
> > >> which
> > >>> is KStream.print(KeyValueMapper<K, V, String>)" with the actual
> added
> > >>> overloaded functions in the wiki page?
> > >>>
> > >>>
> > >>> Guozhang
> > >>>
> > >>> On Mon, May 22, 2017 at 12:21 AM, James Chain <
> > james.chain1990@gmail.com
> > >>>
> > >>> wrote:
> > >>>
> > >>>> Hi All,
> > >>>>
> > >>>> I want to start this KIP to argument KStream.print().
> > >>>> This vote is already started.
> > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >>>> 160+-+Augment+KStream.print%28%29+to+allow+users+pass+in+
> > >>>> extra+parameters+in+the+printed+string
> > >>>>
> > >>>> Thanks,
> > >>>>
> > >>>> James Chien
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>
> > >>
> > >
> > >
> >
> > --
> -Cheers
>
> Jeyhun
>
Re: KIP-160 - Augment KStream.print() to allow users pass in extra
parameters in the printed string
Posted by Jeyhun Karimov <je...@gmail.com>.
I agree with Matthias's comment. Constructing RecordContext with more
metadata seems more feasible for me.
Cheers,
Jeyun
On Mon, Jun 5, 2017 at 7:47 AM Matthias J. Sax <ma...@confluent.io>
wrote:
> Not with the scope of the current discussion.
>
> So far, we discuss to add `RecordContext`, but the context object we use
> could also provide some more metadata. I see no reason why not to expose
> the node name there. We already expose TaskId vie `ProcessorContext`. We
> could also add thread name. IMHO, this would be better than dictating
> any prefix.
>
> Thoughts?
>
>
> -Matthias
>
> On 6/4/17 9:03 PM, Guozhang Wang wrote:
> > Matthias,
> >
> > I think even with KIP-159 users would not be able to access the processor
> > node name right?
> >
> > Guozhang
> >
> > On Thu, Jun 1, 2017 at 10:28 PM, Matthias J. Sax <ma...@confluent.io>
> > wrote:
> >
> >> Thanks for the KIP.
> >>
> >> Two comments:
> >> - I think we should include #writeAsText()
> >> - I am not sure if we should use
> >>
> >>> "[" + this.streamName + "]: " + mapper.apply(keyToPrint, valueToPrint)
> >>
> >> in case a mapper is provided. This still dictates a fixed prefix a user
> >> might not want to have (what contradicts or at least limits the scope of
> >> this new functionality). Considering he current discussion of KIP-159, a
> >> user would be able to access the stream name within the provided mapper
> >> and add it if they wish anyway, and thus, I don't think we should force
> >> this format.
> >>
> >>
> >>
> >> -Matthias
> >>
> >>
> >>
> >> On 5/30/17 1:38 PM, Guozhang Wang wrote:
> >>> Overall +1. One comment about the wiki itself:
> >>>
> >>> Could you replace the general description of "Argument KStream.print()
> >> which
> >>> is KStream.print(KeyValueMapper<K, V, String>)" with the actual added
> >>> overloaded functions in the wiki page?
> >>>
> >>>
> >>> Guozhang
> >>>
> >>> On Mon, May 22, 2017 at 12:21 AM, James Chain <
> james.chain1990@gmail.com
> >>>
> >>> wrote:
> >>>
> >>>> Hi All,
> >>>>
> >>>> I want to start this KIP to argument KStream.print().
> >>>> This vote is already started.
> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>>> 160+-+Augment+KStream.print%28%29+to+allow+users+pass+in+
> >>>> extra+parameters+in+the+printed+string
> >>>>
> >>>> Thanks,
> >>>>
> >>>> James Chien
> >>>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
>
> --
-Cheers
Jeyhun
Re: KIP-160 - Augment KStream.print() to allow users pass in extra
parameters in the printed string
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Not with the scope of the current discussion.
So far, we discuss to add `RecordContext`, but the context object we use
could also provide some more metadata. I see no reason why not to expose
the node name there. We already expose TaskId vie `ProcessorContext`. We
could also add thread name. IMHO, this would be better than dictating
any prefix.
Thoughts?
-Matthias
On 6/4/17 9:03 PM, Guozhang Wang wrote:
> Matthias,
>
> I think even with KIP-159 users would not be able to access the processor
> node name right?
>
> Guozhang
>
> On Thu, Jun 1, 2017 at 10:28 PM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
>> Thanks for the KIP.
>>
>> Two comments:
>> - I think we should include #writeAsText()
>> - I am not sure if we should use
>>
>>> "[" + this.streamName + "]: " + mapper.apply(keyToPrint, valueToPrint)
>>
>> in case a mapper is provided. This still dictates a fixed prefix a user
>> might not want to have (what contradicts or at least limits the scope of
>> this new functionality). Considering he current discussion of KIP-159, a
>> user would be able to access the stream name within the provided mapper
>> and add it if they wish anyway, and thus, I don't think we should force
>> this format.
>>
>>
>>
>> -Matthias
>>
>>
>>
>> On 5/30/17 1:38 PM, Guozhang Wang wrote:
>>> Overall +1. One comment about the wiki itself:
>>>
>>> Could you replace the general description of "Argument KStream.print()
>> which
>>> is KStream.print(KeyValueMapper<K, V, String>)" with the actual added
>>> overloaded functions in the wiki page?
>>>
>>>
>>> Guozhang
>>>
>>> On Mon, May 22, 2017 at 12:21 AM, James Chain <james.chain1990@gmail.com
>>>
>>> wrote:
>>>
>>>> Hi All,
>>>>
>>>> I want to start this KIP to argument KStream.print().
>>>> This vote is already started.
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>> 160+-+Augment+KStream.print%28%29+to+allow+users+pass+in+
>>>> extra+parameters+in+the+printed+string
>>>>
>>>> Thanks,
>>>>
>>>> James Chien
>>>>
>>>
>>>
>>>
>>
>>
>
>
Re: KIP-160 - Augment KStream.print() to allow users pass in extra
parameters in the printed string
Posted by Guozhang Wang <wa...@gmail.com>.
Matthias,
I think even with KIP-159 users would not be able to access the processor
node name right?
Guozhang
On Thu, Jun 1, 2017 at 10:28 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> Thanks for the KIP.
>
> Two comments:
> - I think we should include #writeAsText()
> - I am not sure if we should use
>
> > "[" + this.streamName + "]: " + mapper.apply(keyToPrint, valueToPrint)
>
> in case a mapper is provided. This still dictates a fixed prefix a user
> might not want to have (what contradicts or at least limits the scope of
> this new functionality). Considering he current discussion of KIP-159, a
> user would be able to access the stream name within the provided mapper
> and add it if they wish anyway, and thus, I don't think we should force
> this format.
>
>
>
> -Matthias
>
>
>
> On 5/30/17 1:38 PM, Guozhang Wang wrote:
> > Overall +1. One comment about the wiki itself:
> >
> > Could you replace the general description of "Argument KStream.print()
> which
> > is KStream.print(KeyValueMapper<K, V, String>)" with the actual added
> > overloaded functions in the wiki page?
> >
> >
> > Guozhang
> >
> > On Mon, May 22, 2017 at 12:21 AM, James Chain <james.chain1990@gmail.com
> >
> > wrote:
> >
> >> Hi All,
> >>
> >> I want to start this KIP to argument KStream.print().
> >> This vote is already started.
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> 160+-+Augment+KStream.print%28%29+to+allow+users+pass+in+
> >> extra+parameters+in+the+printed+string
> >>
> >> Thanks,
> >>
> >> James Chien
> >>
> >
> >
> >
>
>
--
-- Guozhang