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