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