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
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
signature.asc
Description: OpenPGP digital signature
