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