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

Reply via email to