Thanks for all the work on this KIP Damian. Both `Produced` and `Joined` have a `with` method accepting all parameters, but `Consumed` doesn't. Should we add one for consistency?
Thanks, Bill On Wed, Aug 23, 2017 at 4:15 AM, Damian Guy <damian....@gmail.com> wrote: > KIP has been updated. thanks > > On Wed, 23 Aug 2017 at 09:10 Damian Guy <damian....@gmail.com> wrote: > > > Hi Matthias, > > > > > >> KStream: > >> leftJoin and outerJoin for KStream/KTable join should not have > >> `JoinWindows` parameter > >> > >> Thanks! > > > > > >> > >> Nit: TopologyBuilder -> Topology > >> > >> Ack > > > > > >> Nit: new class Serialized list static method #with twice > >> > >> Ack > > > > > >> WindowedKStream -> for consistency we should either have GroupedKStream > >> or KWindowedStream... (similar argument for SessionWindowedKStream) > >> > >> We can't rename KGroupedStream -> GroupedKStream without breaking > > compatibility. So we are stuck with it for now. Hopefully in the future > we > > can rename KGroupedStream to GroupedKStream. > > > > > >> > >> KGroupedStream > >> -> why do we use a different name for `sessionWindowedBy()` -- seems to > >> be cleaner to call both methods `windowedBy()` > >> > >> > > I beg to differ that it is cleaner either way! > > > > > >> > >> StreamsBuilder#stream -> parameter order is confusing... We should have > >> Pattern as second parameter to align both methods. > >> > >> Ack > > > > > >> StreamsBuilder#table/globalTable -> move parameter `Consumed` as first > >> parameter to align with `#stream` > >> > >> > >> Ack > > > >> Produced#with(Serde, Serde) > >> Produced#with(StreamPartitioner, Serde, Serde) > >> -> should StreamPartitioner be the third argument instead of the first? > >> > >> Sure > > > >> > >> Consumed: > >> Why do we need 3 different names for the 3 static methods? I would all > >> of them just call `with()`. Current names sound clumsy to me. And a > >> plain `with()` also aligns with the naming of static methods of other > >> classes. > >> > > > > I disagree that the names sound clumsy! But yes they should be aligned > > with the others. > > > > > >> > >> > >> I guess we are also deprecation a bunch of method for > >> KStream/KTable/KGroupedStream/KGroupedTable and should mention which > >> one? There is just one sentence "Deprecate the existing overloads.", but > >> we don't deprecate all existing once. I personally don't care to much if > >> we spell deprecated method out explicitly, but right now it's not > >> consistent as we only list methods we add. > >> > >> > > > >> Should we deprecate `StateStoreSupplier`? > >> > > Yep > > > >> > >> > >> -Matthias > >> > >> > >> > >> On 8/22/17 6:55 AM, Damian Guy wrote: > >> > I've just updated the KIP with some additional changes targeted at > >> > StreamsBuilder > >> > > >> > Thanks, > >> > Damian > >> > > >> > On Thu, 10 Aug 2017 at 12:59 Damian Guy <damian....@gmail.com> wrote: > >> > > >> >> > >> >>> Got it, thanks. > >> >>> > >> >>> Does it still make sense to have one static constructors for each > >> spec, > >> >>> with one constructor having only one parameter to make it more > usable, > >> >>> i.e. > >> >>> as a user I do not need to give all parameters if I only want to > >> override > >> >>> one of them? Maybe we can just name the constructors as `with` but > >> I'm not > >> >>> sure if Java distinguish: > >> >>> > >> >>> public static <K, V> Produced<K, V> with(final Serde<K> keySerde) > >> >>> public static <K, V> Produced<K, V> with(final Serde<V> valueSerde) > >> >>> > >> >>> as two function signatures. > >> >>> > >> >>> > >> >> No that won't work. That is why we have all options, i.e., on Produce > >> >> public static <K, V> Produced<K, V> with(final Serde<K> keySerde, > >> final Serde<V> > >> >> valueSerde) > >> >> public static <K, V> Produced<K, V> with(final StreamPartitioner<K, > V> > >> >> partitioner, final Serde<K> keySerde, final Serde<V> valueSerde) > >> >> public static <K, V> Produced<K, V> keySerde(final Serde<K> keySerde) > >> >> public static <K, V> Produced<K, V> valueSerde(final Serde<V> > >> valueSerde) > >> >> public static <K, V> Produced<K, V> streamPartitioner(final > >> StreamPartitioner<K, > >> >> V> partitioner) > >> >> > >> >> So if you only want to use one you can just use the function that > takes > >> >> one argument. > >> >> > >> >>> > >> >>> Guozhang > >> >>> > >> >>> > >> >>> On Wed, Aug 9, 2017 at 6:20 AM, Damian Guy <damian....@gmail.com> > >> wrote: > >> >>> > >> >>>> On Tue, 8 Aug 2017 at 20:11 Guozhang Wang <wangg...@gmail.com> > >> wrote: > >> >>>> > >> >>>>> Damian, > >> >>>>> > >> >>>>> Thanks for the proposal, I had a few comments on the APIs: > >> >>>>> > >> >>>>> 1. Printed#withFile seems not needed, as users should always spec > if > >> >>> it > >> >>>> is > >> >>>>> to sysOut or to File at the beginning. In addition as a second > >> >>> thought, I > >> >>>>> think serdes are not useful for prints anyways since we assume > >> >>> `toString` > >> >>>>> is provided except for byte arrays, in which we will special > handle > >> >>> it. > >> >>>>> > >> >>>>> > >> >>>> +1 > >> >>>> > >> >>>> > >> >>>>> Another comment about Printed in general is it differs with other > >> >>> options > >> >>>>> that it is a required option than optional one, since it includes > >> >>>> toSysOut > >> >>>>> / toFile specs; what are the pros and cons for including these two > >> in > >> >>> the > >> >>>>> option and hence make it a required option than leaving them at > the > >> >>> API > >> >>>>> layer and make Printed as optional for mapper / label only? > >> >>>>> > >> >>>>> > >> >>>> It isn't required as we will still have the no-arg print() which > will > >> >>> just > >> >>>> go to sysout as it does now. > >> >>>> > >> >>>> > >> >>>>> > >> >>>>> 2.1 KStream#through / to > >> >>>>> > >> >>>>> We should have an overloaded function without Produced? > >> >>>>> > >> >>>> > >> >>>> Yes - we already have those so they are not part of the KIP, i.e, > >> >>>> through(topic) > >> >>>> > >> >>>> > >> >>>>> > >> >>>>> 2.2 KStream#groupBy / groupByKey > >> >>>>> > >> >>>>> We should have an overloaded function without Serialized? > >> >>>>> > >> >>>> > >> >>>> Yes, as above > >> >>>> > >> >>>>> > >> >>>>> 2.3 KGroupedStream#count / reduce / aggregate > >> >>>>> > >> >>>>> We should have an overloaded function without Materialized? > >> >>>>> > >> >>>> > >> >>>> As above > >> >>>> > >> >>>>> > >> >>>>> 2.4 KStream#join > >> >>>>> > >> >>>>> We should have an overloaded function without Joined? > >> >>>>> > >> >>>> > >> >>>> as above > >> >>>> > >> >>>>> > >> >>>>> > >> >>>>> 2.5 Each of KTable's operators: > >> >>>>> > >> >>>>> We should have an overloaded function without Produced / > Serialized > >> / > >> >>>>> Materialized? > >> >>>>> > >> >>>>> > >> >>>> as above > >> >>>> > >> >>>> > >> >>>>> > >> >>>>> > >> >>>>> 3.1 Produced: the static functions have overlaps, which seems not > >> >>>>> necessary. I'd suggest jut having the following three static with > >> >>> another > >> >>>>> three similar member functions: > >> >>>>> > >> >>>>> public static <K, V> Produced<K, V> withKeySerde(final Serde<K> > >> >>> keySerde) > >> >>>>> > >> >>>>> public static <K, V> Produced<K, V> withValueSerde(final Serde<V> > >> >>>>> valueSerde) > >> >>>>> > >> >>>>> public static <K, V> Produced<K, V> withStreamPartitioner(final > >> >>>>> StreamPartitioner<K, V> partitioner) > >> >>>>> > >> >>>>> The key idea is that by using the same function name string for > >> static > >> >>>>> constructor and member functions, users do not need to remember > what > >> >>> are > >> >>>>> the differences but can call these functions with any ordering > they > >> >>> want, > >> >>>>> and later calls on the same spec will win over early calls. > >> >>>>> > >> >>>>> > >> >>>> That would be great if java supported it, but it doesn't. You can't > >> have > >> >>>> static an member functions with the same signature. > >> >>>> > >> >>>> > >> >>>>> > >> >>>>> 3.2 Serialized: similarly > >> >>>>> > >> >>>>> public static <K, V> Serialized<K, V> withKeySerde(final Serde<K> > >> >>>> keySerde) > >> >>>>> > >> >>>>> public static <K, V> Serialized<K, V> withValueSerde(final > Serde<V> > >> >>>>> valueSerde) > >> >>>>> > >> >>>>> public Serialized<K, V> withKeySerde(final Serde<K> keySerde) > >> >>>>> > >> >>>>> public Serialized<K, V> withValueSerde(final Serde valueSerde) > >> >>>>> > >> >>>> > >> >>>> as above > >> >>>> > >> >>>> > >> >>>>> > >> >>>>> Also it has a final Serde<V> otherValueSerde in one of its static > >> >>>>> constructor, it that intentional? > >> >>>>> > >> >>>> > >> >>>> Nope: thanks. > >> >>>> > >> >>>>> > >> >>>>> 3.3. Joined: similarly, keep the static constructor signatures the > >> >>> same > >> >>>> as > >> >>>>> its corresponding member fields. > >> >>>>> > >> >>>>> > >> >>>> As above > >> >>>> > >> >>>> > >> >>>>> 3.4 Materialized: it is a bit special, and I think we can keep its > >> >>> static > >> >>>>> constructors with only two `as` as they are today.K > >> >>>>> > >> >>>>> > >> >>>> 4. Is there any modifications on StateStoreSupplier? Is it replaced > >> by > >> >>>>> BytesStoreSupplier? Seems some more descriptions are lacking here. > >> >>> Also > >> >>>> in > >> >>>>> > >> >>>>> > >> >>>> No modifications to StateStoreSupplier. It is superseceded by > >> >>>> BytesStoreSupplier. > >> >>>> > >> >>>> > >> >>>> > >> >>>>> public static <K, V, S extends StateStore> Materialized<K, V, S> > >> >>>>> as(final StateStoreSupplier<S> > >> >>>>> supplier) > >> >>>>> > >> >>>>> Is the parameter in type of BytesStoreSupplier? > >> >>>>> > >> >>>> > >> >>>> Yep - thanks > >> >>>> > >> >>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> Guozhang > >> >>>>> > >> >>>>> > >> >>>>> On Thu, Jul 27, 2017 at 5:26 AM, Damian Guy <damian....@gmail.com > > > >> >>>> wrote: > >> >>>>> > >> >>>>>> Updated link: > >> >>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >> >>>>>> 182%3A+Reduce+Streams+DSL+overloads+and+allow+easier+ > >> >>>>>> use+of+custom+storage+engines > >> >>>>>> > >> >>>>>> Thanks, > >> >>>>>> Damian > >> >>>>>> > >> >>>>>> On Thu, 27 Jul 2017 at 13:09 Damian Guy <damian....@gmail.com> > >> >>> wrote: > >> >>>>>> > >> >>>>>>> Hi, > >> >>>>>>> > >> >>>>>>> I've put together a KIP to make some changes to the KafkaStreams > >> >>> DSL > >> >>>>> that > >> >>>>>>> will hopefully allow us to: > >> >>>>>>> 1) reduce the explosion of overloads > >> >>>>>>> 2) add new features without having to continue adding more > >> >>> overloads > >> >>>>>>> 3) provide simpler ways for people to use custom storage engines > >> >>> and > >> >>>>> wrap > >> >>>>>>> them with logging, caching etc if desired > >> >>>>>>> 4) enable per-operator caching rather than global caching > without > >> >>>>> having > >> >>>>>>> to resort to supplying a StateStoreSupplier when you just want > to > >> >>>> turn > >> >>>>>>> caching off. > >> >>>>>>> > >> >>>>>>> The KIP is here: > >> >>>>>>> https://cwiki.apache.org/confluence/pages/viewpage. > >> >>>>>> action?pageId=73631309 > >> >>>>>>> > >> >>>>>>> Thanks, > >> >>>>>>> Damian > >> >>>>>>> > >> >>>>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> >>>>> -- > >> >>>>> -- Guozhang > >> >>>>> > >> >>>> > >> >>> > >> >>> > >> >>> > >> >>> -- > >> >>> -- Guozhang > >> >>> > >> >> > >> > > >> > >> >
