Hello folks,
Please let me know if you have further feedbacks; if there is no more
feedbacks I'm going to start the voting thread soon.
Guozhang
On Wed, May 16, 2018 at 8:31 AM, Guozhang Wang <wangg...@gmail.com> wrote:
> I have thought about exposing record context as well, and in the end I
> decided to piggy-back it with KIP-159. And if we want to indeed reuse the
> class it will be:
>
> ```
> public interface RichKeyValueMapper<K, V, VR> {
> VR apply(final K key, final V value, final RecordContext
> recordContext);
> }
> ```
>
>
>
> Guozhang
>
>
> On Tue, May 15, 2018 at 10:04 PM, Matthias J. Sax <matth...@confluent.io>
> wrote:
>
>> Just my 2 cents:
>>
>> I am fine with `KeyValueMapper` (+1 for code reusage) -- the JavaDocs
>> will explain what the `KeyValueMapper` is supposed to do, ie, extract
>> and return the sink topic name from the key-value pair.
>>
>> A side remark though: do we think that accessing key/value is
>> sufficient? Or should we provide access to the full metadata? We could
>> also do this with KIP-159 of course -- but this would come earliest in
>> 2.1. As an alternative we could add a `TopicNameExtractor` to expose the
>> whole record context. The advantage would be, that we don't need to
>> change it via KIP-159 later again. WDYT?
>>
>> -Matthias
>>
>> On 5/15/18 5:57 PM, Bill Bejeck wrote:
>> > Thanks for the KIP Guozhang, it's a +1 for me.
>> >
>> > As for re-using the KeyValueMapper for choosing the topic, I am on the
>> > fence, a more explicitly named class would be more clear, but I'm not
>> sure
>> > it's worth a new class that will primarily perform the same actions as
>> the
>> > KeyValueMapper.
>> >
>> > Thanks,
>> > Bill
>> >
>> > On Tue, May 15, 2018 at 5:52 PM, Guozhang Wang <wangg...@gmail.com>
>> wrote:
>> >
>> >> Hello John:
>> >>
>> >> * As for the type superclass, it is mainly for allowing super class
>> serdes.
>> >> More details can be found here:
>> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> >> 100+-+Relax+Type+constraints+in+Kafka+Streams+API
>> >>
>> >> * I may have slight preference on reusing existing classes but I think
>> most
>> >> of my rationales are quite subjective. Personally I do not find `self
>> >> documenting` worth a new class but I can be convinced if people have
>> other
>> >> motivations doing it :)
>> >>
>> >>
>> >> Guozhang
>> >>
>> >>
>> >> On Tue, May 15, 2018 at 11:19 AM, John Roesler <j...@confluent.io>
>> wrote:
>> >>
>> >>> Thanks for the KIP, Guozhang.
>> >>>
>> >>> It looks good overall to me; I just have one question:
>> >>> * Why do we bound the generics of KVMapper in KStream to be
>> >> superclass-of-K
>> >>> and superclass-of-V instead of exactly K and V, as in Topology? I
>> might
>> >> be
>> >>> thinking about it wrong, but that seems backwards to me. If anything,
>> >>> bounding to be a subclass-of-K or subclass-of-V would seem right to
>> me.
>> >>>
>> >>> One extra thought: I agree that KVMapper<K,V,String /*topic name*/>
>> is an
>> >>> applicable type for extracting the topic name, but I wonder what the
>> >> value
>> >>> of reusing the KVMapper for this purpose is. Would defining a new
>> class,
>> >>> say TopicNameExtractor<K,V>, provide the same functionality while
>> being a
>> >>> bit more self-documenting?
>> >>>
>> >>> Thanks,
>> >>> -John
>> >>>
>> >>> On Tue, May 15, 2018 at 12:32 AM, Guozhang Wang <wangg...@gmail.com>
>> >>> wrote:
>> >>>
>> >>>> Hello folks,
>> >>>>
>> >>>> I'd like to start a discussion on adding dynamic routing
>> functionality
>> >> in
>> >>>> Streams sink node. I.e. users do not need to specify the topic name
>> at
>> >>>> compilation time but can dynamically determine which topic to send to
>> >>> based
>> >>>> on each record's key value pairs. Please find a KIP here:
>> >>>>
>> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> >>>> 303%3A+Add+Dynamic+Routing+in+Streams+Sink
>> >>>>
>> >>>> Any feedbacks are highly appreciated.
>> >>>>
>> >>>> Thanks!
>> >>>>
>> >>>> -- Guozhang
>> >>>>
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> -- Guozhang
>> >>
>> >
>>
>>
>
>
> --
> -- Guozhang
>
--
-- Guozhang
