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....@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 <e...@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....@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 <nishanth...@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 <bbej...@gmail.com> > wrote: > >> > > >> > > Thanks for the KIP! > >> > > > >> > > +1 > >> > > > >> > > -Bill > >> > > > >> > > On Thu, Jul 26, 2018 at 2:39 AM Guozhang Wang <wangg...@gmail.com> > >> > wrote: > >> > > > >> > > > +1 > >> > > > > >> > > > On Wed, Jul 25, 2018 at 11:13 PM, Matthias J. Sax < > >> > matth...@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 < > >> > > > nishanth...@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
