Recasting my +1
On Mon, May 21, 2018 at 3:38 PM, Guozhang Wang <wangg...@gmail.com> wrote:
> Thanks Matthias, will do.
>
> On Mon, May 21, 2018 at 3:35 PM, Matthias J. Sax <matth...@confluent.io>
> wrote:
>
> > Thanks. I am fine with changing the `StreamPartitioner` interface
> directly.
> >
> > Can you add the idea bout `DynamicStreamPartitioner` to "rejected
> > alternative" section. Thx.
> >
> >
> > recasting +1 (binding)
> >
> >
> > -Matthias
> >
> >
> > On 5/21/18 3:04 PM, Guozhang Wang wrote:
> > > Hello Matthias,
> > >
> > > I've tried it out on the PR, the implementation should be fine but one
> > > concern I had is that, as you may also realize users of
> > > DynamicStreamPartitioner needs to implement two interface functions,
> with
> > > and without the topic name if it is extending from StreamPartitioner;
> we
> > > could also let it to not extend from StreamPartition so it has one
> > function
> > > only but then we'd need Produced to have two functions allowing
> > > StreamPartitioner and DynamicStreamPartitioner. Thinking about the pros
> > and
> > > cons I'm think it may be better to just change the interface of
> > > StreamPartitioner itself, since even without dynamic routing, allowing
> > the
> > > topic name could let users to give one partitioner implementation that
> > > branch on the topic names other than having one partitioner per topic.
> > >
> > >
> > > Guozhang
> > >
> > >
> > > On Mon, May 21, 2018 at 11:56 AM, Matthias J. Sax <
> matth...@confluent.io
> > >
> > > wrote:
> > >
> > >> I think that the risk of the change is moderate as I expect most
> people
> > >> to use the DefaultStreamPartitioner.
> > >>
> > >> However, there would still be possibility to define a new interface
> > >> instead of changing the old:
> > >>
> > >>> public interface DynamicStreamPartitioner<K, V> {
> > >>> Integer partition(String topic, K key, V value, int
> numPartitions);
> > >>> }
> > >>
> > >> The newly added methods `Topology#addSink` and `KStream#to` would take
> > >> this new interface instead of the old.
> > >>
> > >> Maybe `DynamicStreamPartitioner` must extend `StreamPartitioner` to
> make
> > >> runtime code work though...
> > >>
> > >> WDYT?
> > >>
> > >> -Matthias
> > >>
> > >> On 5/21/18 11:47 AM, Guozhang Wang wrote:
> > >>> Hello everyone,
> > >>>
> > >>> While implementing the PR for this KIP I realized there is once place
> > >> which
> > >>> we should consider modifying on public APIs as well:
> > >>> StreamPartitioner#partition, to add the topic name string. Note it
> will
> > >> be
> > >>> a incompatible change that requires users who have customized
> > >>> StreamPartitioner implementations.
> > >>>
> > >>> I've updated the wiki page of KIP-303, please recast your vote on
> this
> > >>> thread. Thanks!!!
> > >>>
> > >>>
> > >>> Guozhang
> > >>>
> > >>>
> > >>> On Thu, May 17, 2018 at 3:15 PM, John Roesler <j...@confluent.io>
> > wrote:
> > >>>
> > >>>> +1 non-binding
> > >>>>
> > >>>> On Thu, May 17, 2018 at 4:44 PM, Matthias J. Sax <
> > matth...@confluent.io
> > >>>
> > >>>> wrote:
> > >>>>
> > >>>>> +1 (binding)
> > >>>>>
> > >>>>>
> > >>>>> On 5/17/18 12:18 PM, Ted Yu wrote:
> > >>>>>> +1
> > >>>>>> -------- Original message --------From: Gwen Shapira <
> > >>>> g...@confluent.io>
> > >>>>> Date: 5/17/18 11:53 AM (GMT-08:00) To: dev <dev@kafka.apache.org
> >
> > >>>>> Subject: Re: [VOTE] KIP-303: Add Dynamic Routing Support in Kafka
> > >>>> Streams'
> > >>>>> Topology Sink
> > >>>>>> Yay, its about time :)
> > >>>>>>
> > >>>>>> +1
> > >>>>>>
> > >>>>>> On Thu, May 17, 2018 at 12:38 PM, Guozhang Wang <
> wangg...@gmail.com
> > >
> > >>>>> wrote:
> > >>>>>>
> > >>>>>>> Hello folks,
> > >>>>>>>
> > >>>>>>> I'd like to start a voting thread on adding dynamic routing
> > >>>>> functionality
> > >>>>>>> in Streams sink node. Please find a KIP here:
> > >>>>>>>
> > >>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >>>>>>> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> And the PR itself ready for review as well under KAFKA-4936:
> > >>>>>>>
> > >>>>>>> https://github.com/apache/kafka/pull/5018
> > >>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> Thanks!
> > >>>>>>> -- Guozhang
> > >>>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>
> > >>
> > >
> > >
> >
> >
>
>
> --
> -- Guozhang
>
--
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
<http://www.confluent.io/blog>
