kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Damian Guy <damian....@gmail.com>
Subject Re: [Vote] KIP-321: Update TopologyDescription to better represent Source and Sink Nodes
Date Thu, 02 Aug 2018 10:13:26 GMT
You have 3 binding votes, so i'll defer to the others.

On Thu, 2 Aug 2018 at 04:41 Nishanth Pradeep <nishanthp21@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 <wangguoz@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 <damian.guy@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 <damian.guy@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 <bbejeck@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
>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message