You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Nishanth Pradeep <ni...@gmail.com> on 2018/07/26 02:24:31 UTC
[Vote] KIP-321: Update TopologyDescription to better represent Source
and Sink Nodes
Hello,
I'm calling a vote for KIP-321:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+TopologyDescription+to+better+represent+Source+and+Sink+Nodes
Best,
Nishanth Pradeep
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Nishanth Pradeep <ni...@gmail.com>.
KIP 321 has passed.
Here is the vote break down:
Binding:
- Matthias J. Sax
- Guozhang Wong
- Ewen Cheslack-Postava
Non-Binding:
- Ted Yu
- Bill Bejeck
- Damian Guy
Thanks to all those who voted and provided feedback!
Best,
Nishanth Pradeep
On Thu, Aug 2, 2018 at 12:42 PM Matthias J. Sax <ma...@confluent.io>
wrote:
> I agree with Guozhang.
>
> Breaking compatibility is not acceptable.
>
> If we want the change to use `Optional`, we should deprecate the current
> method and explain that it return type will change in next major release
> 3.0.0 and create a ticket for this change that we can tackle when time
> comes.
>
>
>
> -Matthias
>
> On 8/2/18 9:10 AM, Guozhang Wang wrote:
> > I think leaving the current return value to be null-able is okay, as long
> > as it is well documented in java doc.
> >
> >
> > Guozhang
> >
> > On Thu, Aug 2, 2018 at 3:13 AM, Damian Guy <da...@gmail.com> wrote:
> >
> >> You have 3 binding votes, so i'll defer to the others.
> >>
> >> On Thu, 2 Aug 2018 at 04:41 Nishanth Pradeep <ni...@gmail.com>
> >> wrote:
> >>
> >>> The only issue I see with this is that Sink#topic would also need to be
> >>> Optional as was pointed out already. Since Sink#topic is a preexisting
> >>> method, changing its return type would break backwards compatibility.
> >>>
> >>> On the other hand, it might be worth it to rip that bandaid now.
> >>>
> >>> Best,
> >>> Nishanth Pradeep
> >>>
> >>> On Wed, Aug 1, 2018 at 11:56 AM Guozhang Wang <wa...@gmail.com>
> >> wrote:
> >>>
> >>>> For source node, only one of `Set<topics> topicsSet` and `TopicPattern
> >>>> topicPattern()` will be specified by the user. Similarly for sink
> node,
> >>>> only one of `String` and `TopicNameExtractor` will be specified by the
> >>>> user. Although I've not seen Nishanth's updated PR, I think when it is
> >> not
> >>>> specified today we will return null in that case.
> >>>>
> >>>> If we want to improve on this situation with Optional, we'd need to do
> >> it
> >>>> on all of these functions. Also note that for `Source#toString()` and
> >>>> `Sink#toString()` we should only include the specified field in the
> >>>> resulted representation.
> >>>>
> >>>>
> >>>> Guozhang
> >>>>
> >>>> On Wed, Aug 1, 2018 at 5:08 AM, Damian Guy <da...@gmail.com>
> >> wrote:
> >>>>
> >>>>> Ewen - no as I don't believe they are never null. Whereas the
> >>>>> topicNameExtractor method returns null if it is the default extractor
> >> or
> >>>>> the extractor. So i think this would be better to be optional as it
> is
> >>>>> optionally returning a TopicNameExtractor
> >>>>>
> >>>>> On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <
> ewen@confluent.io
> >>>
> >>>>> wrote:
> >>>>>
> >>>>>> Generally +1 (binding)
> >>>>>>
> >>>>>> It would be helpful to just provide the full, updated interfaces in
> >>>> the
> >>>>>> KIP and mark things as new with comments if needed. I had to go back
> >>>> and
> >>>>>> read the discussion thread to make sure I was understanding the
> >> intent
> >>>>>> correctly.
> >>>>>>
> >>>>>> Damian -- if we make that Optional, shouldn't the methods on Source
> >>>> also
> >>>>>> be Optional types?
> >>>>>>
> >>>>>> -Ewen
> >>>>>>
> >>>>>> On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com>
> >>>>> wrote:
> >>>>>>
> >>>>>>> Hi Nishanth,
> >>>>>>>
> >>>>>>> I have one nit on the KIP. I think the topicNameExtractor method
> >>>> should
> >>>>>>> return Optional<TopicNameExtractor> rather than null.
> >>>>>>> Sorry I'm late here.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Damian
> >>>>>>>
> >>>>>>> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <
> >> nishanthp21@gmail.com
> >>>>>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> We need one more binding vote.
> >>>>>>>>
> >>>>>>>> Binding Votes:
> >>>>>>>>
> >>>>>>>> - Matthias J. Sax
> >>>>>>>> - Guozhang Wong
> >>>>>>>>
> >>>>>>>> Community Votes:
> >>>>>>>>
> >>>>>>>> - Bill Bejeck
> >>>>>>>> - Ted Yu
> >>>>>>>>
> >>>>>>>> Best,
> >>>>>>>> Nishanth Pradeep
> >>>>>>>>
> >>>>>>>> On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com>
> >>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Thanks for the KIP!
> >>>>>>>>>
> >>>>>>>>> +1
> >>>>>>>>>
> >>>>>>>>> -Bill
> >>>>>>>>>
> >>>>>>>>> On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <
> >>>> wangguoz@gmail.com>
> >>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>> +1
> >>>>>>>>>>
> >>>>>>>>>> On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
> >>>>>>>> matthias@confluent.io
> >>>>>>>>>>
> >>>>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> +1 (binding)
> >>>>>>>>>>>
> >>>>>>>>>>> -Matthias
> >>>>>>>>>>>
> >>>>>>>>>>> On 7/25/18 7:47 PM, Ted Yu wrote:
> >>>>>>>>>>>> +1
> >>>>>>>>>>>>
> >>>>>>>>>>>> On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> >>>>>>>>>> nishanthp21@gmail.com>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Hello,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I'm calling a vote for KIP-321:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>>>> 321%3A+Update+
> >>>>>>>>>>> TopologyDescription+to+better+represent+Source+and+Sink+
> >> Nodes
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Best,
> >>>>>>>>>>>>> Nishanth Pradeep
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> --
> >>>>>>>>>> -- Guozhang
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>>
> >>>>
> >>>> --
> >>>> -- Guozhang
> >>>>
> >>>
> >>
> >
> >
> >
>
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by "Matthias J. Sax" <ma...@confluent.io>.
I agree with Guozhang.
Breaking compatibility is not acceptable.
If we want the change to use `Optional`, we should deprecate the current
method and explain that it return type will change in next major release
3.0.0 and create a ticket for this change that we can tackle when time
comes.
-Matthias
On 8/2/18 9:10 AM, Guozhang Wang wrote:
> I think leaving the current return value to be null-able is okay, as long
> as it is well documented in java doc.
>
>
> Guozhang
>
> On Thu, Aug 2, 2018 at 3:13 AM, Damian Guy <da...@gmail.com> wrote:
>
>> You have 3 binding votes, so i'll defer to the others.
>>
>> On Thu, 2 Aug 2018 at 04:41 Nishanth Pradeep <ni...@gmail.com>
>> wrote:
>>
>>> The only issue I see with this is that Sink#topic would also need to be
>>> Optional as was pointed out already. Since Sink#topic is a preexisting
>>> method, changing its return type would break backwards compatibility.
>>>
>>> On the other hand, it might be worth it to rip that bandaid now.
>>>
>>> Best,
>>> Nishanth Pradeep
>>>
>>> On Wed, Aug 1, 2018 at 11:56 AM Guozhang Wang <wa...@gmail.com>
>> wrote:
>>>
>>>> For source node, only one of `Set<topics> topicsSet` and `TopicPattern
>>>> topicPattern()` will be specified by the user. Similarly for sink node,
>>>> only one of `String` and `TopicNameExtractor` will be specified by the
>>>> user. Although I've not seen Nishanth's updated PR, I think when it is
>> not
>>>> specified today we will return null in that case.
>>>>
>>>> If we want to improve on this situation with Optional, we'd need to do
>> it
>>>> on all of these functions. Also note that for `Source#toString()` and
>>>> `Sink#toString()` we should only include the specified field in the
>>>> resulted representation.
>>>>
>>>>
>>>> Guozhang
>>>>
>>>> On Wed, Aug 1, 2018 at 5:08 AM, Damian Guy <da...@gmail.com>
>> wrote:
>>>>
>>>>> Ewen - no as I don't believe they are never null. Whereas the
>>>>> topicNameExtractor method returns null if it is the default extractor
>> or
>>>>> the extractor. So i think this would be better to be optional as it is
>>>>> optionally returning a TopicNameExtractor
>>>>>
>>>>> On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <ewen@confluent.io
>>>
>>>>> wrote:
>>>>>
>>>>>> Generally +1 (binding)
>>>>>>
>>>>>> It would be helpful to just provide the full, updated interfaces in
>>>> the
>>>>>> KIP and mark things as new with comments if needed. I had to go back
>>>> and
>>>>>> read the discussion thread to make sure I was understanding the
>> intent
>>>>>> correctly.
>>>>>>
>>>>>> Damian -- if we make that Optional, shouldn't the methods on Source
>>>> also
>>>>>> be Optional types?
>>>>>>
>>>>>> -Ewen
>>>>>>
>>>>>> On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>>> Hi Nishanth,
>>>>>>>
>>>>>>> I have one nit on the KIP. I think the topicNameExtractor method
>>>> should
>>>>>>> return Optional<TopicNameExtractor> rather than null.
>>>>>>> Sorry I'm late here.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Damian
>>>>>>>
>>>>>>> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <
>> nishanthp21@gmail.com
>>>>>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> We need one more binding vote.
>>>>>>>>
>>>>>>>> Binding Votes:
>>>>>>>>
>>>>>>>> - Matthias J. Sax
>>>>>>>> - Guozhang Wong
>>>>>>>>
>>>>>>>> Community Votes:
>>>>>>>>
>>>>>>>> - Bill Bejeck
>>>>>>>> - Ted Yu
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> Nishanth Pradeep
>>>>>>>>
>>>>>>>> On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com>
>>>>> wrote:
>>>>>>>>
>>>>>>>>> Thanks for the KIP!
>>>>>>>>>
>>>>>>>>> +1
>>>>>>>>>
>>>>>>>>> -Bill
>>>>>>>>>
>>>>>>>>> On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <
>>>> wangguoz@gmail.com>
>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> +1
>>>>>>>>>>
>>>>>>>>>> On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
>>>>>>>> matthias@confluent.io
>>>>>>>>>>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> +1 (binding)
>>>>>>>>>>>
>>>>>>>>>>> -Matthias
>>>>>>>>>>>
>>>>>>>>>>> On 7/25/18 7:47 PM, Ted Yu wrote:
>>>>>>>>>>>> +1
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
>>>>>>>>>> nishanthp21@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm calling a vote for KIP-321:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>>> 321%3A+Update+
>>>>>>>>>>> TopologyDescription+to+better+represent+Source+and+Sink+
>> Nodes
>>>>>>>>>>>>>
>>>>>>>>>>>>> Best,
>>>>>>>>>>>>> Nishanth Pradeep
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> --
>>>>>>>>>> -- Guozhang
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> -- Guozhang
>>>>
>>>
>>
>
>
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Guozhang Wang <wa...@gmail.com>.
I think leaving the current return value to be null-able is okay, as long
as it is well documented in java doc.
Guozhang
On Thu, Aug 2, 2018 at 3:13 AM, Damian Guy <da...@gmail.com> wrote:
> You have 3 binding votes, so i'll defer to the others.
>
> On Thu, 2 Aug 2018 at 04:41 Nishanth Pradeep <ni...@gmail.com>
> wrote:
>
> > The only issue I see with this is that Sink#topic would also need to be
> > Optional as was pointed out already. Since Sink#topic is a preexisting
> > method, changing its return type would break backwards compatibility.
> >
> > On the other hand, it might be worth it to rip that bandaid now.
> >
> > Best,
> > Nishanth Pradeep
> >
> > On Wed, Aug 1, 2018 at 11:56 AM Guozhang Wang <wa...@gmail.com>
> wrote:
> >
> >> For source node, only one of `Set<topics> topicsSet` and `TopicPattern
> >> topicPattern()` will be specified by the user. Similarly for sink node,
> >> only one of `String` and `TopicNameExtractor` will be specified by the
> >> user. Although I've not seen Nishanth's updated PR, I think when it is
> not
> >> specified today we will return null in that case.
> >>
> >> If we want to improve on this situation with Optional, we'd need to do
> it
> >> on all of these functions. Also note that for `Source#toString()` and
> >> `Sink#toString()` we should only include the specified field in the
> >> resulted representation.
> >>
> >>
> >> Guozhang
> >>
> >> On Wed, Aug 1, 2018 at 5:08 AM, Damian Guy <da...@gmail.com>
> wrote:
> >>
> >> > Ewen - no as I don't believe they are never null. Whereas the
> >> > topicNameExtractor method returns null if it is the default extractor
> or
> >> > the extractor. So i think this would be better to be optional as it is
> >> > optionally returning a TopicNameExtractor
> >> >
> >> > On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <ewen@confluent.io
> >
> >> > wrote:
> >> >
> >> > > Generally +1 (binding)
> >> > >
> >> > > It would be helpful to just provide the full, updated interfaces in
> >> the
> >> > > KIP and mark things as new with comments if needed. I had to go back
> >> and
> >> > > read the discussion thread to make sure I was understanding the
> intent
> >> > > correctly.
> >> > >
> >> > > Damian -- if we make that Optional, shouldn't the methods on Source
> >> also
> >> > > be Optional types?
> >> > >
> >> > > -Ewen
> >> > >
> >> > > On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com>
> >> > wrote:
> >> > >
> >> > >> Hi Nishanth,
> >> > >>
> >> > >> I have one nit on the KIP. I think the topicNameExtractor method
> >> should
> >> > >> return Optional<TopicNameExtractor> rather than null.
> >> > >> Sorry I'm late here.
> >> > >>
> >> > >> Thanks,
> >> > >> Damian
> >> > >>
> >> > >> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <
> nishanthp21@gmail.com
> >> >
> >> > >> wrote:
> >> > >>
> >> > >> > We need one more binding vote.
> >> > >> >
> >> > >> > Binding Votes:
> >> > >> >
> >> > >> > - Matthias J. Sax
> >> > >> > - Guozhang Wong
> >> > >> >
> >> > >> > Community Votes:
> >> > >> >
> >> > >> > - Bill Bejeck
> >> > >> > - Ted Yu
> >> > >> >
> >> > >> > Best,
> >> > >> > Nishanth Pradeep
> >> > >> >
> >> > >> > On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com>
> >> > wrote:
> >> > >> >
> >> > >> > > Thanks for the KIP!
> >> > >> > >
> >> > >> > > +1
> >> > >> > >
> >> > >> > > -Bill
> >> > >> > >
> >> > >> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <
> >> wangguoz@gmail.com>
> >> > >> > wrote:
> >> > >> > >
> >> > >> > > > +1
> >> > >> > > >
> >> > >> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
> >> > >> > matthias@confluent.io
> >> > >> > > >
> >> > >> > > > wrote:
> >> > >> > > >
> >> > >> > > > > +1 (binding)
> >> > >> > > > >
> >> > >> > > > > -Matthias
> >> > >> > > > >
> >> > >> > > > > On 7/25/18 7:47 PM, Ted Yu wrote:
> >> > >> > > > > > +1
> >> > >> > > > > >
> >> > >> > > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> >> > >> > > > nishanthp21@gmail.com>
> >> > >> > > > > > wrote:
> >> > >> > > > > >
> >> > >> > > > > >> Hello,
> >> > >> > > > > >>
> >> > >> > > > > >> I'm calling a vote for KIP-321:
> >> > >> > > > > >>
> >> > >> > > > > >>
> >> > >> > > > > >>
> >> > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> > 321%3A+Update+
> >> > >> > > > > TopologyDescription+to+better+represent+Source+and+Sink+
> Nodes
> >> > >> > > > > >>
> >> > >> > > > > >> Best,
> >> > >> > > > > >> Nishanth Pradeep
> >> > >> > > > > >>
> >> > >> > > > > >
> >> > >> > > > >
> >> > >> > > > >
> >> > >> > > >
> >> > >> > > >
> >> > >> > > > --
> >> > >> > > > -- Guozhang
> >> > >> > > >
> >> > >> > >
> >> > >> >
> >> > >>
> >> > >
> >> >
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
>
--
-- Guozhang
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Damian Guy <da...@gmail.com>.
You have 3 binding votes, so i'll defer to the others.
On Thu, 2 Aug 2018 at 04:41 Nishanth Pradeep <ni...@gmail.com> wrote:
> The only issue I see with this is that Sink#topic would also need to be
> Optional as was pointed out already. Since Sink#topic is a preexisting
> method, changing its return type would break backwards compatibility.
>
> On the other hand, it might be worth it to rip that bandaid now.
>
> Best,
> Nishanth Pradeep
>
> On Wed, Aug 1, 2018 at 11:56 AM Guozhang Wang <wa...@gmail.com> wrote:
>
>> For source node, only one of `Set<topics> topicsSet` and `TopicPattern
>> topicPattern()` will be specified by the user. Similarly for sink node,
>> only one of `String` and `TopicNameExtractor` will be specified by the
>> user. Although I've not seen Nishanth's updated PR, I think when it is not
>> specified today we will return null in that case.
>>
>> If we want to improve on this situation with Optional, we'd need to do it
>> on all of these functions. Also note that for `Source#toString()` and
>> `Sink#toString()` we should only include the specified field in the
>> resulted representation.
>>
>>
>> Guozhang
>>
>> On Wed, Aug 1, 2018 at 5:08 AM, Damian Guy <da...@gmail.com> wrote:
>>
>> > Ewen - no as I don't believe they are never null. Whereas the
>> > topicNameExtractor method returns null if it is the default extractor or
>> > the extractor. So i think this would be better to be optional as it is
>> > optionally returning a TopicNameExtractor
>> >
>> > On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <ew...@confluent.io>
>> > wrote:
>> >
>> > > Generally +1 (binding)
>> > >
>> > > It would be helpful to just provide the full, updated interfaces in
>> the
>> > > KIP and mark things as new with comments if needed. I had to go back
>> and
>> > > read the discussion thread to make sure I was understanding the intent
>> > > correctly.
>> > >
>> > > Damian -- if we make that Optional, shouldn't the methods on Source
>> also
>> > > be Optional types?
>> > >
>> > > -Ewen
>> > >
>> > > On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com>
>> > wrote:
>> > >
>> > >> Hi Nishanth,
>> > >>
>> > >> I have one nit on the KIP. I think the topicNameExtractor method
>> should
>> > >> return Optional<TopicNameExtractor> rather than null.
>> > >> Sorry I'm late here.
>> > >>
>> > >> Thanks,
>> > >> Damian
>> > >>
>> > >> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <nishanthp21@gmail.com
>> >
>> > >> wrote:
>> > >>
>> > >> > We need one more binding vote.
>> > >> >
>> > >> > Binding Votes:
>> > >> >
>> > >> > - Matthias J. Sax
>> > >> > - Guozhang Wong
>> > >> >
>> > >> > Community Votes:
>> > >> >
>> > >> > - Bill Bejeck
>> > >> > - Ted Yu
>> > >> >
>> > >> > Best,
>> > >> > Nishanth Pradeep
>> > >> >
>> > >> > On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com>
>> > wrote:
>> > >> >
>> > >> > > Thanks for the KIP!
>> > >> > >
>> > >> > > +1
>> > >> > >
>> > >> > > -Bill
>> > >> > >
>> > >> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <
>> wangguoz@gmail.com>
>> > >> > wrote:
>> > >> > >
>> > >> > > > +1
>> > >> > > >
>> > >> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
>> > >> > matthias@confluent.io
>> > >> > > >
>> > >> > > > wrote:
>> > >> > > >
>> > >> > > > > +1 (binding)
>> > >> > > > >
>> > >> > > > > -Matthias
>> > >> > > > >
>> > >> > > > > On 7/25/18 7:47 PM, Ted Yu wrote:
>> > >> > > > > > +1
>> > >> > > > > >
>> > >> > > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
>> > >> > > > nishanthp21@gmail.com>
>> > >> > > > > > wrote:
>> > >> > > > > >
>> > >> > > > > >> Hello,
>> > >> > > > > >>
>> > >> > > > > >> I'm calling a vote for KIP-321:
>> > >> > > > > >>
>> > >> > > > > >>
>> > >> > > > > >>
>> > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > 321%3A+Update+
>> > >> > > > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
>> > >> > > > > >>
>> > >> > > > > >> Best,
>> > >> > > > > >> Nishanth Pradeep
>> > >> > > > > >>
>> > >> > > > > >
>> > >> > > > >
>> > >> > > > >
>> > >> > > >
>> > >> > > >
>> > >> > > > --
>> > >> > > > -- Guozhang
>> > >> > > >
>> > >> > >
>> > >> >
>> > >>
>> > >
>> >
>>
>>
>>
>> --
>> -- Guozhang
>>
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Nishanth Pradeep <ni...@gmail.com>.
The only issue I see with this is that Sink#topic would also need to be
Optional as was pointed out already. Since Sink#topic is a preexisting
method, changing its return type would break backwards compatibility.
On the other hand, it might be worth it to rip that bandaid now.
Best,
Nishanth Pradeep
On Wed, Aug 1, 2018 at 11:56 AM Guozhang Wang <wa...@gmail.com> wrote:
> For source node, only one of `Set<topics> topicsSet` and `TopicPattern
> topicPattern()` will be specified by the user. Similarly for sink node,
> only one of `String` and `TopicNameExtractor` will be specified by the
> user. Although I've not seen Nishanth's updated PR, I think when it is not
> specified today we will return null in that case.
>
> If we want to improve on this situation with Optional, we'd need to do it
> on all of these functions. Also note that for `Source#toString()` and
> `Sink#toString()` we should only include the specified field in the
> resulted representation.
>
>
> Guozhang
>
> On Wed, Aug 1, 2018 at 5:08 AM, Damian Guy <da...@gmail.com> wrote:
>
> > Ewen - no as I don't believe they are never null. Whereas the
> > topicNameExtractor method returns null if it is the default extractor or
> > the extractor. So i think this would be better to be optional as it is
> > optionally returning a TopicNameExtractor
> >
> > On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <ew...@confluent.io>
> > wrote:
> >
> > > Generally +1 (binding)
> > >
> > > It would be helpful to just provide the full, updated interfaces in the
> > > KIP and mark things as new with comments if needed. I had to go back
> and
> > > read the discussion thread to make sure I was understanding the intent
> > > correctly.
> > >
> > > Damian -- if we make that Optional, shouldn't the methods on Source
> also
> > > be Optional types?
> > >
> > > -Ewen
> > >
> > > On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com>
> > wrote:
> > >
> > >> Hi Nishanth,
> > >>
> > >> I have one nit on the KIP. I think the topicNameExtractor method
> should
> > >> return Optional<TopicNameExtractor> rather than null.
> > >> Sorry I'm late here.
> > >>
> > >> Thanks,
> > >> Damian
> > >>
> > >> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <ni...@gmail.com>
> > >> wrote:
> > >>
> > >> > We need one more binding vote.
> > >> >
> > >> > Binding Votes:
> > >> >
> > >> > - Matthias J. Sax
> > >> > - Guozhang Wong
> > >> >
> > >> > Community Votes:
> > >> >
> > >> > - Bill Bejeck
> > >> > - Ted Yu
> > >> >
> > >> > Best,
> > >> > Nishanth Pradeep
> > >> >
> > >> > On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com>
> > wrote:
> > >> >
> > >> > > Thanks for the KIP!
> > >> > >
> > >> > > +1
> > >> > >
> > >> > > -Bill
> > >> > >
> > >> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wangguoz@gmail.com
> >
> > >> > wrote:
> > >> > >
> > >> > > > +1
> > >> > > >
> > >> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
> > >> > matthias@confluent.io
> > >> > > >
> > >> > > > wrote:
> > >> > > >
> > >> > > > > +1 (binding)
> > >> > > > >
> > >> > > > > -Matthias
> > >> > > > >
> > >> > > > > On 7/25/18 7:47 PM, Ted Yu wrote:
> > >> > > > > > +1
> > >> > > > > >
> > >> > > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> > >> > > > nishanthp21@gmail.com>
> > >> > > > > > wrote:
> > >> > > > > >
> > >> > > > > >> Hello,
> > >> > > > > >>
> > >> > > > > >> I'm calling a vote for KIP-321:
> > >> > > > > >>
> > >> > > > > >>
> > >> > > > > >>
> > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 321%3A+Update+
> > >> > > > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> > >> > > > > >>
> > >> > > > > >> Best,
> > >> > > > > >> Nishanth Pradeep
> > >> > > > > >>
> > >> > > > > >
> > >> > > > >
> > >> > > > >
> > >> > > >
> > >> > > >
> > >> > > > --
> > >> > > > -- Guozhang
> > >> > > >
> > >> > >
> > >> >
> > >>
> > >
> >
>
>
>
> --
> -- Guozhang
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Guozhang Wang <wa...@gmail.com>.
For source node, only one of `Set<topics> topicsSet` and `TopicPattern
topicPattern()` will be specified by the user. Similarly for sink node,
only one of `String` and `TopicNameExtractor` will be specified by the
user. Although I've not seen Nishanth's updated PR, I think when it is not
specified today we will return null in that case.
If we want to improve on this situation with Optional, we'd need to do it
on all of these functions. Also note that for `Source#toString()` and
`Sink#toString()` we should only include the specified field in the
resulted representation.
Guozhang
On Wed, Aug 1, 2018 at 5:08 AM, Damian Guy <da...@gmail.com> wrote:
> Ewen - no as I don't believe they are never null. Whereas the
> topicNameExtractor method returns null if it is the default extractor or
> the extractor. So i think this would be better to be optional as it is
> optionally returning a TopicNameExtractor
>
> On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <ew...@confluent.io>
> wrote:
>
> > Generally +1 (binding)
> >
> > It would be helpful to just provide the full, updated interfaces in the
> > KIP and mark things as new with comments if needed. I had to go back and
> > read the discussion thread to make sure I was understanding the intent
> > correctly.
> >
> > Damian -- if we make that Optional, shouldn't the methods on Source also
> > be Optional types?
> >
> > -Ewen
> >
> > On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com>
> wrote:
> >
> >> Hi Nishanth,
> >>
> >> I have one nit on the KIP. I think the topicNameExtractor method should
> >> return Optional<TopicNameExtractor> rather than null.
> >> Sorry I'm late here.
> >>
> >> Thanks,
> >> Damian
> >>
> >> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <ni...@gmail.com>
> >> wrote:
> >>
> >> > We need one more binding vote.
> >> >
> >> > Binding Votes:
> >> >
> >> > - Matthias J. Sax
> >> > - Guozhang Wong
> >> >
> >> > Community Votes:
> >> >
> >> > - Bill Bejeck
> >> > - Ted Yu
> >> >
> >> > Best,
> >> > Nishanth Pradeep
> >> >
> >> > On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com>
> wrote:
> >> >
> >> > > Thanks for the KIP!
> >> > >
> >> > > +1
> >> > >
> >> > > -Bill
> >> > >
> >> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wa...@gmail.com>
> >> > wrote:
> >> > >
> >> > > > +1
> >> > > >
> >> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
> >> > matthias@confluent.io
> >> > > >
> >> > > > wrote:
> >> > > >
> >> > > > > +1 (binding)
> >> > > > >
> >> > > > > -Matthias
> >> > > > >
> >> > > > > On 7/25/18 7:47 PM, Ted Yu wrote:
> >> > > > > > +1
> >> > > > > >
> >> > > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> >> > > > nishanthp21@gmail.com>
> >> > > > > > wrote:
> >> > > > > >
> >> > > > > >> Hello,
> >> > > > > >>
> >> > > > > >> I'm calling a vote for KIP-321:
> >> > > > > >>
> >> > > > > >>
> >> > > > > >>
> >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 321%3A+Update+
> >> > > > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> >> > > > > >>
> >> > > > > >> Best,
> >> > > > > >> Nishanth Pradeep
> >> > > > > >>
> >> > > > > >
> >> > > > >
> >> > > > >
> >> > > >
> >> > > >
> >> > > > --
> >> > > > -- Guozhang
> >> > > >
> >> > >
> >> >
> >>
> >
>
--
-- Guozhang
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Damian Guy <da...@gmail.com>.
Ewen - no as I don't believe they are never null. Whereas the
topicNameExtractor method returns null if it is the default extractor or
the extractor. So i think this would be better to be optional as it is
optionally returning a TopicNameExtractor
On Tue, 31 Jul 2018 at 23:01 Ewen Cheslack-Postava <ew...@confluent.io>
wrote:
> Generally +1 (binding)
>
> It would be helpful to just provide the full, updated interfaces in the
> KIP and mark things as new with comments if needed. I had to go back and
> read the discussion thread to make sure I was understanding the intent
> correctly.
>
> Damian -- if we make that Optional, shouldn't the methods on Source also
> be Optional types?
>
> -Ewen
>
> On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com> wrote:
>
>> Hi Nishanth,
>>
>> I have one nit on the KIP. I think the topicNameExtractor method should
>> return Optional<TopicNameExtractor> rather than null.
>> Sorry I'm late here.
>>
>> Thanks,
>> Damian
>>
>> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <ni...@gmail.com>
>> wrote:
>>
>> > We need one more binding vote.
>> >
>> > Binding Votes:
>> >
>> > - Matthias J. Sax
>> > - Guozhang Wong
>> >
>> > Community Votes:
>> >
>> > - Bill Bejeck
>> > - Ted Yu
>> >
>> > Best,
>> > Nishanth Pradeep
>> >
>> > On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com> wrote:
>> >
>> > > Thanks for the KIP!
>> > >
>> > > +1
>> > >
>> > > -Bill
>> > >
>> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wa...@gmail.com>
>> > wrote:
>> > >
>> > > > +1
>> > > >
>> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
>> > matthias@confluent.io
>> > > >
>> > > > wrote:
>> > > >
>> > > > > +1 (binding)
>> > > > >
>> > > > > -Matthias
>> > > > >
>> > > > > On 7/25/18 7:47 PM, Ted Yu wrote:
>> > > > > > +1
>> > > > > >
>> > > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
>> > > > nishanthp21@gmail.com>
>> > > > > > wrote:
>> > > > > >
>> > > > > >> Hello,
>> > > > > >>
>> > > > > >> I'm calling a vote for KIP-321:
>> > > > > >>
>> > > > > >>
>> > > > > >>
>> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+
>> > > > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
>> > > > > >>
>> > > > > >> Best,
>> > > > > >> Nishanth Pradeep
>> > > > > >>
>> > > > > >
>> > > > >
>> > > > >
>> > > >
>> > > >
>> > > > --
>> > > > -- Guozhang
>> > > >
>> > >
>> >
>>
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
Generally +1 (binding)
It would be helpful to just provide the full, updated interfaces in the KIP
and mark things as new with comments if needed. I had to go back and read
the discussion thread to make sure I was understanding the intent correctly.
Damian -- if we make that Optional, shouldn't the methods on Source also be
Optional types?
-Ewen
On Mon, Jul 30, 2018 at 11:13 PM Damian Guy <da...@gmail.com> wrote:
> Hi Nishanth,
>
> I have one nit on the KIP. I think the topicNameExtractor method should
> return Optional<TopicNameExtractor> rather than null.
> Sorry I'm late here.
>
> Thanks,
> Damian
>
> On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <ni...@gmail.com>
> wrote:
>
> > We need one more binding vote.
> >
> > Binding Votes:
> >
> > - Matthias J. Sax
> > - Guozhang Wong
> >
> > Community Votes:
> >
> > - Bill Bejeck
> > - Ted Yu
> >
> > Best,
> > Nishanth Pradeep
> >
> > On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com> wrote:
> >
> > > Thanks for the KIP!
> > >
> > > +1
> > >
> > > -Bill
> > >
> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wa...@gmail.com>
> > wrote:
> > >
> > > > +1
> > > >
> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
> > matthias@confluent.io
> > > >
> > > > wrote:
> > > >
> > > > > +1 (binding)
> > > > >
> > > > > -Matthias
> > > > >
> > > > > On 7/25/18 7:47 PM, Ted Yu wrote:
> > > > > > +1
> > > > > >
> > > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> > > > nishanthp21@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > >> Hello,
> > > > > >>
> > > > > >> I'm calling a vote for KIP-321:
> > > > > >>
> > > > > >>
> > > > > >>
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+
> > > > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> > > > > >>
> > > > > >> Best,
> > > > > >> Nishanth Pradeep
> > > > > >>
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> >
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Damian Guy <da...@gmail.com>.
Hi Nishanth,
I have one nit on the KIP. I think the topicNameExtractor method should
return Optional<TopicNameExtractor> rather than null.
Sorry I'm late here.
Thanks,
Damian
On Tue, 31 Jul 2018 at 01:14 Nishanth Pradeep <ni...@gmail.com> wrote:
> We need one more binding vote.
>
> Binding Votes:
>
> - Matthias J. Sax
> - Guozhang Wong
>
> Community Votes:
>
> - Bill Bejeck
> - Ted Yu
>
> Best,
> Nishanth Pradeep
>
> On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com> wrote:
>
> > Thanks for the KIP!
> >
> > +1
> >
> > -Bill
> >
> > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wa...@gmail.com>
> wrote:
> >
> > > +1
> > >
> > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <
> matthias@confluent.io
> > >
> > > wrote:
> > >
> > > > +1 (binding)
> > > >
> > > > -Matthias
> > > >
> > > > On 7/25/18 7:47 PM, Ted Yu wrote:
> > > > > +1
> > > > >
> > > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> > > nishanthp21@gmail.com>
> > > > > wrote:
> > > > >
> > > > >> Hello,
> > > > >>
> > > > >> I'm calling a vote for KIP-321:
> > > > >>
> > > > >>
> > > > >>
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+
> > > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> > > > >>
> > > > >> Best,
> > > > >> Nishanth Pradeep
> > > > >>
> > > > >
> > > >
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Nishanth Pradeep <ni...@gmail.com>.
We need one more binding vote.
Binding Votes:
- Matthias J. Sax
- Guozhang Wong
Community Votes:
- Bill Bejeck
- Ted Yu
Best,
Nishanth Pradeep
On Fri, Jul 27, 2018 at 10:02 AM Bill Bejeck <bb...@gmail.com> wrote:
> Thanks for the KIP!
>
> +1
>
> -Bill
>
> On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wa...@gmail.com> wrote:
>
> > +1
> >
> > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <matthias@confluent.io
> >
> > wrote:
> >
> > > +1 (binding)
> > >
> > > -Matthias
> > >
> > > On 7/25/18 7:47 PM, Ted Yu wrote:
> > > > +1
> > > >
> > > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> > nishanthp21@gmail.com>
> > > > wrote:
> > > >
> > > >> Hello,
> > > >>
> > > >> I'm calling a vote for KIP-321:
> > > >>
> > > >>
> > > >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+
> > > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> > > >>
> > > >> Best,
> > > >> Nishanth Pradeep
> > > >>
> > > >
> > >
> > >
> >
> >
> > --
> > -- Guozhang
> >
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Bill Bejeck <bb...@gmail.com>.
Thanks for the KIP!
+1
-Bill
On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wa...@gmail.com> wrote:
> +1
>
> On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > +1 (binding)
> >
> > -Matthias
> >
> > On 7/25/18 7:47 PM, Ted Yu wrote:
> > > +1
> > >
> > > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <
> nishanthp21@gmail.com>
> > > wrote:
> > >
> > >> Hello,
> > >>
> > >> I'm calling a vote for KIP-321:
> > >>
> > >>
> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+
> > TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> > >>
> > >> Best,
> > >> Nishanth Pradeep
> > >>
> > >
> >
> >
>
>
> --
> -- Guozhang
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Guozhang Wang <wa...@gmail.com>.
+1
On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> +1 (binding)
>
> -Matthias
>
> On 7/25/18 7:47 PM, Ted Yu wrote:
> > +1
> >
> > On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <ni...@gmail.com>
> > wrote:
> >
> >> Hello,
> >>
> >> I'm calling a vote for KIP-321:
> >>
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+
> TopologyDescription+to+better+represent+Source+and+Sink+Nodes
> >>
> >> Best,
> >> Nishanth Pradeep
> >>
> >
>
>
--
-- Guozhang
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by "Matthias J. Sax" <ma...@confluent.io>.
+1 (binding)
-Matthias
On 7/25/18 7:47 PM, Ted Yu wrote:
> +1
>
> On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <ni...@gmail.com>
> wrote:
>
>> Hello,
>>
>> I'm calling a vote for KIP-321:
>>
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+TopologyDescription+to+better+represent+Source+and+Sink+Nodes
>>
>> Best,
>> Nishanth Pradeep
>>
>
Re: [Vote] KIP-321: Update TopologyDescription to better represent
Source and Sink Nodes
Posted by Ted Yu <yu...@gmail.com>.
+1
On Wed, Jul 25, 2018 at 7:24 PM Nishanth Pradeep <ni...@gmail.com>
wrote:
> Hello,
>
> I'm calling a vote for KIP-321:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-321%3A+Update+TopologyDescription+to+better+represent+Source+and+Sink+Nodes
>
> Best,
> Nishanth Pradeep
>