Breaking API without a deprecation cycle doesn't seem good. Are we sure
about this?
Ismael
On Mon, 21 May 2018, 15:04 Guozhang Wang, <wangg...@gmail.com> 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
>
