Hi Jeyhun, thanks for starting the VOTE thread. I did make one more pass over the KIP before casting my vote and I saw that the KIP still contains backward incompatible change introducing `ValueTransformerCommon`.
I think, that for this case, it is not worth breaking compatibility. We should have two independent interface and duplicate init() and close() (note, with KIP-138 that got merged already, we don't need `punctuate()` for ValueTransformerWithKey) -Matthias On 6/14/17 3:03 PM, Matthias J. Sax wrote: > I have no strong opinion, but it seems that at least InitializerWithKey > with be helpful if you want to have different start values for different > keys (even if I cannot come up with a use case example why one wanted to > do this...). Otherwise, there is just the "completeness" argument, that > is not too strong either. > > > -Matthias > > On 6/14/17 2:03 PM, Guozhang Wang wrote: >> I'm not particularly concerning that we should NEVER break compatibility; >> in fact if we think that is worthwhile (i.e. small impact with large >> benefit) I think we can break compatibility as long as we have not removed >> the compatibility annotations from Streams. All I was saying is that if we >> decided to go this way we need to make sure this is mentioned in the >> upgrade guidance. >> >> Regarding the scope I'm still trying to solicit opinions regarding >> ReducerWithKey and InitializerWithKey; to me they are not necessarily to be >> included. >> >> >> Guozhang >> >> >> On Wed, Jun 14, 2017 at 5:22 AM, Jeyhun Karimov <je.kari...@gmail.com> >> wrote: >> >>> Hi, >>> >>> I introduced ValueTransformerCommon just to combine common methods of >>> ValueTransformer and ValueTransformerWithKey and avoid copy-paste. >>> I am aware of this issue and I agree that this needs users to compile the >>> code and therefore is not backwards compatible. When I saw this issue, I >>> thought the degree of incompatibility is small (the public APIs are the >>> same, users just need to recompile their code), so we can trade more >>> maintainable code in this case. I have to comments/solutions: >>> >>> 1. Basically we can remove ValueTransformerCommon class and return >>> ValueTransformer to its original form, which means there will be no issues >>> with backwards-compatibility. We just copy and past the methods inside >>> ValueTransformerCommon to ValueTransformerWithKey and maybe in future >>> releases, we can introduce ValueTransformerCommon. >>> >>> 2. I have some doubts about Matthias's proposal. >>> If we extent withKey interface from original one as you mentioned in >>> previous email, then we have to deal with >>> ValueTransformer.transform(V value) method. As a result, inside withKey >>> interface we will have two transforms. Even if we make it abstract class, >>> user still have an access to play with both transform() methods. I think >>> this should not be allowed and seems "hacky" to me. Actually this was one >>> reason why I created withKey classes independently from original >>> interfaces. >>> >>> Of course, you can correct me if I am wrong. >>> >>> >>> Cheers, >>> Jeyhun >>> >>> >>> >>> On Tue, Jun 13, 2017 at 7:42 PM Matthias J. Sax <matth...@confluent.io> >>> wrote: >>> >>>> I agree with Guozhang's second point. This change does not seem backward >>>> compatible. >>>> >>>> As we don't have to support lambdas, it might be the easiest thing to >>>> just extend the current interface: >>>> >>>>> public interface ValueTransformerWithKey<K, V, VR> extends >>>> ValueTransformer<VR> >>>> >>>> When plugging the topology together, we can check if we get the >>>> `withKey` variant and use a corresponding runtime class for execution, >>>> so we get only a single time check. Thus, for the `withKey` variant, the >>>> will be a `transfrom(V value)` method, but we will never call it. >>>> >>>> Maybe we could make `ValueTransformerWithKey` an abstract class with a >>>> `final` no-op implemenation of `transform(V value)` ? >>>> >>>> >>>> -Matthias >>>> >>>> >>>> On 6/6/17 4:58 PM, Guozhang Wang wrote: >>>>> Jeyhun, Matthias: >>>>> >>>>> Thanks for the explanation, I overlooked the repartition argument >>>>> previously. >>>>> >>>>> 1) Based on that argument I'm convinced of having ValueMapperWithKey / >>>>> ValueJoinerWithKey / ValueTransformerWithKey; though I'm still not >>>>> convinced with ReducerWithKey and InitializerWithKey since for the >>> former >>>>> it can be covered with `aggregate` completely and with latter I have >>> seen >>>>> little use cases with it. >>>>> >>>>> 2) Another comment is on public interface ValueTransformer<V, VR> >>> extends >>>>> ValueTransformerCommon<VR>: >>>>> >>>>> I think changing the interface to extend from a new interface is not >>>> binary >>>>> compatible though source compatible, i.e. users still need to recompile >>>>> their code though no need to make code changes. We may need to mention >>>> that >>>>> in the upgrade path if we want to keep it that way. >>>>> >>>>> Guozhang >>>>> >>>>> On Mon, Jun 5, 2017 at 2:28 PM, Jeyhun Karimov <je.kari...@gmail.com> >>>> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> >>>>>> Sorry for late reply. Just to make everybody in sync, the current >>>> version >>>>>> of KIP supports lambdas. "withKey" (ValueMapperWithKey) interfaces are >>>>>> independent, meaning they do not extend from "withoutKey" >>> (ValueMapper) >>>>>> interfaces. >>>>>> >>>>>> >>>>>> I agree with Guozhang, and I am personally a bit reluctant to increase >>>>>> overloaded methods in public APIs but it seems this is only way to >>> solve >>>>>> all related jira issues. >>>>>> However, the most overloaded methods will be with ValueJoiner type, >>>> which >>>>>> will be with ValueJoinerWithKey with new overloaded methods. Other >>>>>> interfaces require mostly 1 extra overload. >>>>>> >>>>>> >>>>>>>> I would suggest not doing it if user pop it up, but rather >>> suggesting >>>>>> them >>>>>>>> to use `map` >>>>>> >>>>>> I agree with Matthias as the core idea of this KIP was to collect all >>>>>> related jira issues and propose one-shot solution for all. Afterwards, >>>> we >>>>>> broke its scope into 2 KIPs (149 and 159). >>>>>> >>>>>> Cheers, >>>>>> Jeyhun >>>>>> >>>>>> >>>>>> >>>>>> On Mon, Jun 5, 2017 at 7:55 AM Matthias J. Sax <matth...@confluent.io >>>> >>>>>> wrote: >>>>>> >>>>>>> I guess I missunderstood you. Your are right that overloading the >>>> method >>>>>>> would also work. As both interfaces will be independent from each >>>> other, >>>>>>> there should be no problem. >>>>>>> >>>>>>> The initial proposal was to use >>>>>>> >>>>>>>> interface ValueMapperWithKey extends ValueMapper >>>>>>> >>>>>>> and this would break Lambda. We totally missed, that we don't need >>> new >>>>>>> methods but only only overlaods. Great you point this out! I was not >>>>>>> quite happy with the newly added methods. >>>>>>> >>>>>>>>> I would suggest not doing it if user pop it up, but rather >>> suggesting >>>>>>> them >>>>>>>>> to use `map` >>>>>>> >>>>>>> But this might introduce unnecessary re-partitioning for downstream >>>>>>> operators. I don't thinks that a good suggestion. But if we only add >>>> new >>>>>>> overloads for mapValue(), flatMapValue() etc, I don't see a big issue >>>>>>> with "overstaffing" the API (what is btw a very valid concern). >>>>>>> >>>>>>> >>>>>>> -Matthias >>>>>>> >>>>>>> >>>>>>> On 6/4/17 10:37 PM, Guozhang Wang wrote: >>>>>>>> On Sun, Jun 4, 2017 at 8:41 PM, Matthias J. Sax < >>>> matth...@confluent.io >>>>>>> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> We started with this proposal but it does not allow for Lambdas (in >>>>>> case >>>>>>>>> you want key access). Do you think preserving Lambdas is not >>>> important >>>>>>>>> enough for this case -- I agree that there is some price to be paid >>>> to >>>>>>>>> keep Lambdas. >>>>>>>>> >>>>>>>> >>>>>>>> Not sure if I understand this comment.. My main point is not on >>>>>> changing >>>>>>>> the proposal but just reduce it scope to `ValueJoinerWithKey` only; >>>> and >>>>>>>> even if we want to keep them all, I'd suggest we just implement the >>>>>> added >>>>>>>> APIs by using the `KeyValueMapper` for `ValueMapperWithKeys`, etc, >>>>>> which >>>>>>>> seems simpler to me. Does that affect lambda expression usage? >>>>>>>> >>>>>>>>> >>>>>>>>> About API consistency: I can't remember a concrete user request to >>>>>> have >>>>>>>>> key access in all operators. But I am pretty sure, if we only add >>> it >>>>>> for >>>>>>>>> some, this request will pop up quickly. IMHO, it's better to do it >>>> all >>>>>>>>> in one shot. (But I am not strict about it if most people think we >>>>>>>>> should limit it to what was requested by users.) >>>>>>>>> >>>>>>>>> >>>>>>>> I would suggest not doing it if user pop it up, but rather >>> suggesting >>>>>>> them >>>>>>>> to use `map` etc directly but set the key unchanged rather than >>>>>>> providing a >>>>>>>> new operator for it. To me some syntax sugars like this seems of >>> less >>>>>>>> valuable than others (like print / writeAsText / foreach that are >>> all >>>>>>>> depending on peek), and keeping adding them will just make the DSL >>> too >>>>>>>> “overstaffed”. >>>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> -Matthias >>>>>>>>> >>>>>>>>> On 6/4/17 6:23 PM, Guozhang Wang wrote: >>>>>>>>>> With KIP-159, the added "valueMapperWithKey" and >>>>>>>>> "valueTransformerWithKey" >>>>>>>>>> along seem to be just adding a few syntax-sugars? E.g. we can >>> simply >>>>>>>>>> implement: >>>>>>>>>> >>>>>>>>>> mapValues(ValueMapperWithKey mapperWithKey) >>>>>>>>>> >>>>>>>>>> as >>>>>>>>>> >>>>>>>>>> map((k, v) -> (k, mapperWithKey(k, v)) >>>>>>>>>> >>>>>>>>>> ---------------------- >>>>>>>>>> >>>>>>>>>> I'm not sure how many users would really need such syntax sugars, >>>> and >>>>>>>>> even >>>>>>>>>> they do, we can support them easily as the above implementations; >>>>>>>>>> >>>>>>>>>> Similarly for "ReducerWithKey", it can be implemented as >>>>>> `Aggregator<K, >>>>>>>>> V, >>>>>>>>>> V>`, and people who needs key access can just use `aggregate` >>>>>> directly. >>>>>>>>>> >>>>>>>>>> The function which I think is really of valuable is >>>>>>> `ValueJoinerWithKey`, >>>>>>>>>> and that seems to be the original request from users while others >>>> are >>>>>>>>>> coming from "API consistency" concerns. Personally I'd prefer only >>>>>> keep >>>>>>>>> the >>>>>>>>>> last one (`InitializerWithKey` might also have some value, but I >>>> have >>>>>>> not >>>>>>>>>> seen people widely requesting it in their DSL usage yet; if there >>> is >>>>>> a >>>>>>>>>> common request we can keep it in this KIP as well). WDYT? >>>>>>>>>> >>>>>>>>>> Guozhang >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Sun, May 28, 2017 at 10:16 AM, Jeyhun Karimov < >>>>>> je.kari...@gmail.com >>>>>>>> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Thanks for clarification Matthias, now everything is clear. >>>>>>>>>>> >>>>>>>>>>> On Sun, May 28, 2017 at 6:21 PM Matthias J. Sax < >>>>>>> matth...@confluent.io> >>>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>>> I don't think we can drop ValueTransformerSupplier. If you don't >>>>>> have >>>>>>>>> an >>>>>>>>>>>> supplier, you only get a single instance of your function. But >>> for >>>>>> a >>>>>>>>>>>> stateful transformation, we need multiple instances (one for >>> each >>>>>>> task) >>>>>>>>>>>> of ValueTransformer. >>>>>>>>>>>> >>>>>>>>>>>> We don't need suppliers for functions like "ValueMapper" etc >>>>>> because >>>>>>>>>>>> those are stateless and thus, we can reuse a single instance >>> over >>>>>>>>>>>> multiple tasks -- but we can't do this for ValueTransformer (and >>>>>>>>>>> similar). >>>>>>>>>>>> >>>>>>>>>>>> Btw: This reminds me about KIP-159: with regard to the >>>> RichFunction >>>>>>> we >>>>>>>>>>>> might need a supplier pattern, too. (I'll comment on the other >>>>>>> thread, >>>>>>>>>>>> too.) >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> -Matthias >>>>>>>>>>>> >>>>>>>>>>>> On 5/28/17 5:45 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>> Hi, >>>>>>>>>>>>> >>>>>>>>>>>>> I updated KIP. >>>>>>>>>>>>> Just to avoid misunderstanding, I meant deprecating >>>>>>>>>>>> ValueTransformerSupplier >>>>>>>>>>>>> and I am ok with ValueTransformer. >>>>>>>>>>>>> So instead of using ValueTransformerSupplier can't we directly >>>> use >>>>>>>>>>>>> ValueTransformer >>>>>>>>>>>>> or ValueTransformerWithKey? >>>>>>>>>>>>> >>>>>>>>>>>>> Btw, in current design all features of ValueTransformer will be >>>>>>>>>>> available >>>>>>>>>>>>> in ValueTransformerWithKey interface. >>>>>>>>>>>>> >>>>>>>>>>>>> Cheers, >>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>> >>>>>>>>>>>>> On Sun, May 28, 2017 at 6:15 AM Matthias J. Sax < >>>>>>>>> matth...@confluent.io >>>>>>>>>>>> >>>>>>>>>>>>> wrote: >>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks Jeyhun. >>>>>>>>>>>>>> >>>>>>>>>>>>>> About ValueTransformer: I don't think we can remove it. Note, >>>>>>>>>>>>>> ValueTransformer allows to attach a state and also allows to >>>>>>> register >>>>>>>>>>>>>> punctuations. Both those features will not be available via >>>>>>> withKey() >>>>>>>>>>>>>> interfaces. >>>>>>>>>>>>>> >>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> On 5/27/17 1:25 PM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>> Hi Matthias, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Thanks for your comments. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I tested the deep copy approach. It has significant overhead. >>>>>>>>>>>> Especially >>>>>>>>>>>>>>> for "light" and stateless operators it slows down the >>> topology >>>>>>>>>>>>>>> significantly (> 20% ). I think "warning" users about >>>>>>> not-changing >>>>>>>>>>> the >>>>>>>>>>>>>> key >>>>>>>>>>>>>>> is better warning them about possible performance loss. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> About the interfaces, additionally I considered adding >>>>>>>>>>>>>> InitializerWithKey, >>>>>>>>>>>>>>> AggregatorWithKey and ValueTransformerWithKey. I think they >>> are >>>>>>>>>>>> included >>>>>>>>>>>>>> in >>>>>>>>>>>>>>> PR but not in KIP. I will also include them in KIP, sorry my >>>>>> bad. >>>>>>>>>>>>>>> Including ReducerWithKey definitely makes sense. Thanks. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> One thing I want to mention is that, maybe we should >>> deprecate >>>>>>>>>>> methods >>>>>>>>>>>>>> with >>>>>>>>>>>>>>> argument type ValueTransformerSupplier >>>>>>>>> (KStream.transformValues(...)) >>>>>>>>>>>> and >>>>>>>>>>>>>>> and as a whole the ValueTransformerSupplier interface. >>>>>>>>>>>>>>> We can use ValueTransformer/ValueTransformerWithKey type >>>>>> instead >>>>>>>>>>>> without >>>>>>>>>>>>>>> additional supplier layer. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On Thu, May 25, 2017 at 1:07 AM Matthias J. Sax < >>>>>>>>>>> matth...@confluent.io >>>>>>>>>>>>> >>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> One more question: >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Should we add any of >>>>>>>>>>>>>>>> - InitizialierWithKey >>>>>>>>>>>>>>>> - ReducerWithKey >>>>>>>>>>>>>>>> - ValueTransformerWithKey >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> To get consistent/complete API, it might be a good idea. Any >>>>>>>>>>> thoughts? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On 5/24/17 3:47 PM, Matthias J. Sax wrote: >>>>>>>>>>>>>>>>> Jeyhun, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> I was just wondering if you did look into the key-deep-copy >>>>>> idea >>>>>>>>> we >>>>>>>>>>>>>>>>> discussed. I am curious to see what the impact might be. >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On 5/20/17 2:03 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Thanks for your comments. I rethink about including rich >>>>>>>>> functions >>>>>>>>>>>>>> into >>>>>>>>>>>>>>>>>> this KIP. >>>>>>>>>>>>>>>>>> I think once we include rich functions in this KIP and >>> then >>>>>> fix >>>>>>>>>>>>>>>>>> ProcessorContext in another KIP and incorporate with >>>> existing >>>>>>>>> rich >>>>>>>>>>>>>>>>>> functions, the code will not be backwards compatible. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> I see Damian's and your point more clearly now. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Conclusion: we include only withKey interfaces in this KIP >>>> (I >>>>>>>>>>>> updated >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>> KIP), I will try also initiate another KIP for rich >>>>>> functions. >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> On Fri, May 19, 2017 at 10:50 PM Matthias J. Sax < >>>>>>>>>>>>>> matth...@confluent.io >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> With the current KIP, using ValueMapper and >>>>>> ValueMapperWithKey >>>>>>>>>>>>>>>>>>> interfaces, RichFunction seems to be an independent >>> add-on. >>>>>> To >>>>>>>>>>> fix >>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>> original issue to allow key access, RichFunctions are not >>>>>>>>>>> required >>>>>>>>>>>>>>>> IMHO. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> I initially put the RichFunction idea on the table, >>> because >>>>>> I >>>>>>>>> was >>>>>>>>>>>>>>>> hoping >>>>>>>>>>>>>>>>>>> to get a uniform API. And I think, is was good to >>> consider >>>>>>> them >>>>>>>>>>> -- >>>>>>>>>>>>>>>>>>> however, the discussion showed that they are not >>> necessary >>>>>> for >>>>>>>>>>> key >>>>>>>>>>>>>>>>>>> access. Thus, it seems to be better to handle >>> RichFunctions >>>>>> in >>>>>>>>> an >>>>>>>>>>>> own >>>>>>>>>>>>>>>>>>> KIP. The ProcessorContext/RecordContext issues seems to >>> be >>>> a >>>>>>>>> main >>>>>>>>>>>>>>>>>>> challenge for this. And introducing RichFunctions with >>>>>>>>>>>> parameter-less >>>>>>>>>>>>>>>>>>> init() method, seem not to help too much. We would get an >>>>>>>>>>>>>>>> "intermediate" >>>>>>>>>>>>>>>>>>> API that we want to change anyway later on... >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> As you put already much effort into RichFunction, feel >>> free >>>>>> do >>>>>>>>>>> push >>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>> further and start a new KIP (we can do this even in >>>>>> parallel) >>>>>>> -- >>>>>>>>>>> we >>>>>>>>>>>>>>>>>>> don't want to slow you down :) But it make the discussion >>>>>> and >>>>>>>>>>> code >>>>>>>>>>>>>>>>>>> review easier, if we separate both IMHO. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> On 5/19/17 2:25 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>>>>>>> Hi Damian, >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Thanks for your comments. I think providing to users >>>>>>>>> *interface* >>>>>>>>>>>>>>>> rather >>>>>>>>>>>>>>>>>>>> than *abstract class* should be preferred (Matthias also >>>>>>> raised >>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>> issue >>>>>>>>>>>>>>>>>>>> ), anyway I changed the corresponding parts of KIP. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Regarding with passing additional contextual >>> information, >>>> I >>>>>>>>>>> think >>>>>>>>>>>> it >>>>>>>>>>>>>>>> is a >>>>>>>>>>>>>>>>>>>> tradeoff, >>>>>>>>>>>>>>>>>>>> 1) first, we fix the context parameter for *init() >>> *method >>>>>> in >>>>>>>>>>>>>> another >>>>>>>>>>>>>>>> PR >>>>>>>>>>>>>>>>>>>> and solve Rich functions afterwards >>>>>>>>>>>>>>>>>>>> 2) first, we fix the requested issues on jira ([1-3]) >>> with >>>>>>>>>>>> providing >>>>>>>>>>>>>>>>>>>> (not-complete) Rich functions and integrate the context >>>>>>>>>>> parameters >>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>> afterwards (like improvement) >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> To me, the second approach seems more incremental. >>> However >>>>>>> you >>>>>>>>>>> are >>>>>>>>>>>>>>>> right, >>>>>>>>>>>>>>>>>>>> the names might confuse the users. >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/KAFKA-4218 >>>>>>>>>>>>>>>>>>>> [2] https://issues.apache.org/jira/browse/KAFKA-4726 >>>>>>>>>>>>>>>>>>>> [3] https://issues.apache.org/jira/browse/KAFKA-3745 >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> On Fri, May 19, 2017 at 10:42 AM Damian Guy < >>>>>>>>>>> damian....@gmail.com >>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> I see you've removed the `ProcessorContext` from the >>>>>>>>>>> RichFunction >>>>>>>>>>>>>>>> which >>>>>>>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>> good, but why is it a `RichFunction`? I'd have expected >>>> it >>>>>>> to >>>>>>>>>>>> pass >>>>>>>>>>>>>>>> some >>>>>>>>>>>>>>>>>>>>> additional contextual information, i.e., the >>>>>> `RecordContext` >>>>>>>>>>> that >>>>>>>>>>>>>>>>>>> contains >>>>>>>>>>>>>>>>>>>>> just the topic, partition, timestamp, offset. I'm ok >>>> with >>>>>>> it >>>>>>>>>>> not >>>>>>>>>>>>>>>>>>> passing >>>>>>>>>>>>>>>>>>>>> this contextual information, but is the naming >>> incorrect? >>>>>>> I'm >>>>>>>>>>> not >>>>>>>>>>>>>>>> sure, >>>>>>>>>>>>>>>>>>>>> tbh. I'm wondering if we should drop `RichFunctions` >>>> until >>>>>>> we >>>>>>>>>>> can >>>>>>>>>>>>>> do >>>>>>>>>>>>>>>> it >>>>>>>>>>>>>>>>>>>>> properly with the correct context? >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Also, i don't like the abstract classes: >>> RichValueMapper, >>>>>>>>>>>>>>>>>>> RichValueJoiner, >>>>>>>>>>>>>>>>>>>>> RichInitializer etc. Why can't they not just be >>>>>> interfaces? >>>>>>>>>>>>>>>> Generally we >>>>>>>>>>>>>>>>>>>>> should provide users with Intefaces to code against, >>> not >>>>>>>>>>> classes. >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> Thanks, >>>>>>>>>>>>>>>>>>>>> Damian >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> On Fri, 19 May 2017 at 00:50 Jeyhun Karimov < >>>>>>>>>>>> je.kari...@gmail.com> >>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Thanks. I initiated the PR as well, to get a better >>>>>>> overview. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> The only reason that I used abstract class instead of >>>>>>>>>>> interface >>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>> Rich >>>>>>>>>>>>>>>>>>>>>> functions is that in future if we will have some >>>>>>>>>>>>>>>> AbstractRichFunction >>>>>>>>>>>>>>>>>>>>>> abstract classes, >>>>>>>>>>>>>>>>>>>>>> we can easily extend: >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> public abstract class RichValueMapper<K, V, VR> >>>>>> implements >>>>>>>>>>>>>>>>>>>>>> ValueMapperWithKey<K, V, VR>, RichFunction *extends >>>>>>>>>>>>>>>>>>>>> AbstractRichFunction*{ >>>>>>>>>>>>>>>>>>>>>> } >>>>>>>>>>>>>>>>>>>>>> With interfaces we are only limited to interfaces for >>>>>>>>>>>>>> inheritance. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> However, I think we can easily change it (from >>> abstract >>>>>>> class >>>>>>>>>>> -> >>>>>>>>>>>>>>>>>>>>> interface) >>>>>>>>>>>>>>>>>>>>>> if you think interface is a better fit. >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> On Fri, May 19, 2017 at 1:00 AM Matthias J. Sax < >>>>>>>>>>>>>>>> matth...@confluent.io >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Thanks for the update and explanations. The KIP is >>>> quite >>>>>>>>>>>> improved >>>>>>>>>>>>>>>> now >>>>>>>>>>>>>>>>>>>>> -- >>>>>>>>>>>>>>>>>>>>>>> great job! >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> One more question: Why are RichValueMapper etc >>> abstract >>>>>>>>>>> classes >>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>> not >>>>>>>>>>>>>>>>>>>>>>> interfaces? >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> Overall, I like the KIP a lot! >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>> On 5/16/17 7:03 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Thanks for your comments. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> I think supporting Lambdas for `withKey` and >>>>>>>>>>>>>>>> `AbstractRichFunction` >>>>>>>>>>>>>>>>>>>>>>>>> don't go together, as Lambdas are only supported >>> for >>>>>>>>>>>> interfaces >>>>>>>>>>>>>>>>>>>>> AFAIK. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Maybe I misunderstood your comment. >>>>>>>>>>>>>>>>>>>>>>>> *withKey* and and *withOnlyValue* are interfaces. So >>>>>> they >>>>>>>>>>>> don't >>>>>>>>>>>>>>>> have >>>>>>>>>>>>>>>>>>>>>>> direct >>>>>>>>>>>>>>>>>>>>>>>> relation with *AbstractRichFunction*. >>>>>>>>>>>>>>>>>>>>>>>> *withKey* and and *withOnlyValue* interfaces have >>> only >>>>>>> one >>>>>>>>>>>>>>>> method , >>>>>>>>>>>>>>>>>>>>> so >>>>>>>>>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>> can use lambdas. >>>>>>>>>>>>>>>>>>>>>>>> Where does the *AbstractRichFunction* comes to play? >>>>>>> Inside >>>>>>>>>>>> Rich >>>>>>>>>>>>>>>>>>>>>>> functions. >>>>>>>>>>>>>>>>>>>>>>>> And we use Rich functions in 2 places: >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> 1. User doesn't use rich functions. Just regular >>>>>>> *withKey* >>>>>>>>>>> and >>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>> *withOnlyValue* interfaces(both support lambdas) . >>> We >>>>>> get >>>>>>>>>>>> those >>>>>>>>>>>>>>>>>>>>>>> interfaces >>>>>>>>>>>>>>>>>>>>>>>> and wrap into Rich function while building the >>>>>> topology, >>>>>>>>> and >>>>>>>>>>>>>> send >>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>> Processor. >>>>>>>>>>>>>>>>>>>>>>>> 2. User does use rich functions (Rich functions >>>>>> implement >>>>>>>>>>>>>>>> *withKey* >>>>>>>>>>>>>>>>>>>>>>>> interface). As a result no lamdas here by >>> definition. >>>>>> In >>>>>>>>>>> this >>>>>>>>>>>>>>>> case, >>>>>>>>>>>>>>>>>>>>>> while >>>>>>>>>>>>>>>>>>>>>>>> building the topology we do a type check if the >>> object >>>>>>> type >>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>>>>> *withKey* or *RichFunction*. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> So *AbstractRichFunction* is just syntactic sugar >>> for >>>>>>> Rich >>>>>>>>>>>>>>>> functions >>>>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>> does not affect using lambdas. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Thus, if we want to support Lambdas for `withKey`, >>> we >>>>>>> need >>>>>>>>>>> to >>>>>>>>>>>>>>>> have a >>>>>>>>>>>>>>>>>>>>>>>>> interface approach like this >>>>>>>>>>>>>>>>>>>>>>>>> - RichFunction -> only adding init() and close() >>>>>>>>>>>>>>>>>>>>>>>>> - ValueMapper >>>>>>>>>>>>>>>>>>>>>>>>> - ValueMapperWithKey >>>>>>>>>>>>>>>>>>>>>>>>> - RichValueMapper extends ValueMapperWithKey, >>>>>>>>>>> RichFunction >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> As I said above, currently we support lambdas for >>>>>>> *withKey* >>>>>>>>>>>>>>>>>>>>> interfaces >>>>>>>>>>>>>>>>>>>>>> as >>>>>>>>>>>>>>>>>>>>>>>> well. However, I agree with your idea and I will >>>>>> remove >>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>> AbstractRichFunction from the design. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> As an alternative, we could argue, that it is >>>>>> sufficient >>>>>>> to >>>>>>>>>>>>>>>> support >>>>>>>>>>>>>>>>>>>>>>>>> Lambdas for the "plain" API only, but not for any >>>>>>>>> "extended >>>>>>>>>>>>>> API". >>>>>>>>>>>>>>>>>>>>> For >>>>>>>>>>>>>>>>>>>>>>>>> this, RichFunction could add key+init+close and >>>>>>>>>>>>>>>> AbstractRichFunction >>>>>>>>>>>>>>>>>>>>>>>>> would allow to only care about getting the key. >>>>>>>>>>>>>>>>>>>>>>>>> Not sure, which one is better. I don't like the >>> idea >>>>>> of >>>>>>>>>>> more >>>>>>>>>>>>>>>>>>>>>> overloaded >>>>>>>>>>>>>>>>>>>>>>>>> methods to get Lambdas for `withKey` interfaces too >>>>>> much >>>>>>>>>>>>>> because >>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>> have >>>>>>>>>>>>>>>>>>>>>>>>> already so many overlaods. On the other hand, I do >>>> see >>>>>>>>>>> value >>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>>>>> supporting Lambdas for `withKey`. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Just to clarify, with current design we have only >>> one >>>>>>> extra >>>>>>>>>>>>>>>>>>>>> overloaded >>>>>>>>>>>>>>>>>>>>>>>> method per *withOnlyValue* interface: which is >>>>>> *withKey* >>>>>>>>>>>>>> version >>>>>>>>>>>>>>>> of >>>>>>>>>>>>>>>>>>>>>>>> particular interface. >>>>>>>>>>>>>>>>>>>>>>>> We don't need extra overload for Rich function as >>> Rich >>>>>>>>>>>> function >>>>>>>>>>>>>>>>>>>>>>> implements >>>>>>>>>>>>>>>>>>>>>>>> *withKey* interface as a result they have same type. >>>> We >>>>>>>>>>>>>>>> differentiate >>>>>>>>>>>>>>>>>>>>>>> them >>>>>>>>>>>>>>>>>>>>>>>> while building the topology. >>>>>>>>>>>>>>>>>>>>>>>> We supported lambdas for *withKey* APIs because of >>> the >>>>>>>>>>>> comment: >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> @Jeyhun: I did not put any thought into this, but >>> can >>>>>> we >>>>>>>>>>> have >>>>>>>>>>>> a >>>>>>>>>>>>>>>>>>>>> design >>>>>>>>>>>>>>>>>>>>>>>>> that allows for both? Also, with regard to lambdas, >>>> it >>>>>>>>>>> might >>>>>>>>>>>>>> make >>>>>>>>>>>>>>>>>>>>>> sense >>>>>>>>>>>>>>>>>>>>>>>>> to allow for both `V -> newV` and `(K, V) -> newV` >>> ? >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> However, I don't think that this complicates the >>>>>> overall >>>>>>>>>>>> design >>>>>>>>>>>>>>>>>>>>>>>> significantly. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Depending on what we want to support, it might make >>>>>> sense >>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>> include/exclude RichFunctions from this KIP -- and >>>>>> thus, >>>>>>>>>>> this >>>>>>>>>>>>>>>> also >>>>>>>>>>>>>>>>>>>>>>>>> determines if we should have a "ProcessorContext >>> KIP" >>>>>>>>>>> before >>>>>>>>>>>>>>>> driving >>>>>>>>>>>>>>>>>>>>>>>>> this KIP further. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Based on our discussion I think we should keep Rich >>>>>>>>>>> functions >>>>>>>>>>>>>> as I >>>>>>>>>>>>>>>>>>>>>> don't >>>>>>>>>>>>>>>>>>>>>>>> think that they bring extra layer of overhead to >>>>>> library. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Any comments are appreciated. >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 16, 2017 at 12:10 AM Matthias J. Sax < >>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io> >>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun, >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> thanks for the update. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> I think supporting Lambdas for `withKey` and >>>>>>>>>>>>>>>> `AbstractRichFunction` >>>>>>>>>>>>>>>>>>>>>>>>> don't go together, as Lambdas are only supported >>> for >>>>>>>>>>>> interfaces >>>>>>>>>>>>>>>>>>>>> AFAIK. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Thus, if we want to support Lambdas for `withKey`, >>> we >>>>>>> need >>>>>>>>>>> to >>>>>>>>>>>>>>>> have a >>>>>>>>>>>>>>>>>>>>>>>>> interface approach like this >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> - RichFunction -> only adding init() and close() >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> - ValueMapper >>>>>>>>>>>>>>>>>>>>>>>>> - ValueMapperWithKey >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> - RichValueMapper extends ValueMapperWithKey, >>>>>>>>>>> RichFunction >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> For this approach, AbstractRichFunction does not >>> make >>>>>>>>> sense >>>>>>>>>>>>>>>> anymore, >>>>>>>>>>>>>>>>>>>>>> as >>>>>>>>>>>>>>>>>>>>>>>>> the only purpose of `RichFunction` is to allow the >>>>>>>>>>>>>>>> implementation of >>>>>>>>>>>>>>>>>>>>>>>>> init() and close() -- if you don't want those, you >>>>>> would >>>>>>>>>>>>>>>> implement a >>>>>>>>>>>>>>>>>>>>>>>>> different interface (ie, ValueMapperWithKey) >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> As an alternative, we could argue, that it is >>>>>> sufficient >>>>>>>>> to >>>>>>>>>>>>>>>> support >>>>>>>>>>>>>>>>>>>>>>>>> Lambdas for the "plain" API only, but not for any >>>>>>>>> "extended >>>>>>>>>>>>>> API". >>>>>>>>>>>>>>>>>>>>> For >>>>>>>>>>>>>>>>>>>>>>>>> this, RichFunction could add key+init+close and >>>>>>>>>>>>>>>> AbstractRichFunction >>>>>>>>>>>>>>>>>>>>>>>>> would allow to only care about getting the key. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Not sure, which one is better. I don't like the >>> idea >>>>>> of >>>>>>>>>>> more >>>>>>>>>>>>>>>>>>>>>> overloaded >>>>>>>>>>>>>>>>>>>>>>>>> methods to get Lambdas for `withKey` interfaces too >>>>>> much >>>>>>>>>>>>>> because >>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>> have >>>>>>>>>>>>>>>>>>>>>>>>> already so many overlaods. On the other hand, I do >>>> see >>>>>>>>>>> value >>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>>>>> supporting Lambdas for `withKey`. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Depending on what we want to support, it might make >>>>>>> sense >>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>> include/exclude RichFunctions from this KIP -- and >>>>>> thus, >>>>>>>>>>> this >>>>>>>>>>>>>>>> also >>>>>>>>>>>>>>>>>>>>>>>>> determines if we should have a "ProcessorContext >>> KIP" >>>>>>>>>>> before >>>>>>>>>>>>>>>> driving >>>>>>>>>>>>>>>>>>>>>>>>> this KIP further. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Thoughts? >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> On 5/15/17 11:01 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Sorry for super late response. Thanks for your >>>>>>> comments. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> I am not an expert on Lambdas. Can you elaborate a >>>>>>> little >>>>>>>>>>>>>> bit? I >>>>>>>>>>>>>>>>>>>>>> cannot >>>>>>>>>>>>>>>>>>>>>>>>>>> follow the explanation in the KIP to see what the >>>>>>>>> problem >>>>>>>>>>>> is. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> - From [1] says "A functional interface is an >>>>>> interface >>>>>>>>>>> that >>>>>>>>>>>>>> has >>>>>>>>>>>>>>>>>>>>> just >>>>>>>>>>>>>>>>>>>>>>> one >>>>>>>>>>>>>>>>>>>>>>>>>> abstract method, and thus represents a single >>>>>> function >>>>>>>>>>>>>>>> contract". >>>>>>>>>>>>>>>>>>>>>>>>>> So basically once we extend some interface from >>>>>> another >>>>>>>>>>> (in >>>>>>>>>>>>>> our >>>>>>>>>>>>>>>>>>>>> case, >>>>>>>>>>>>>>>>>>>>>>>>>> ValueMapperWithKey from ValueMapper) we cannot use >>>>>>>>> lambdas >>>>>>>>>>>> in >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>> extended >>>>>>>>>>>>>>>>>>>>>>>>>> interface. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Further comments: >>>>>>>>>>>>>>>>>>>>>>>>>>> - The KIP get a little hard to read -- can you >>>>>> maybe >>>>>>>>>>>>>> reformat >>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>> wiki >>>>>>>>>>>>>>>>>>>>>>>>>>> page a little bit? I think using `CodeBlock` >>> would >>>>>>> help. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> - I will work on the KIP. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> - What about KStream-KTable joins? You don't have >>>>>>>>>>> overlaods >>>>>>>>>>>>>>>> added >>>>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>>>>>> them. Why? (Even if I still hope that we don't >>> need >>>>>> to >>>>>>>>>>> add >>>>>>>>>>>>>> any >>>>>>>>>>>>>>>> new >>>>>>>>>>>>>>>>>>>>>>>>>>> overloads) >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> - Actually there are more than one Processor and >>>>>> public >>>>>>>>>>> APIs >>>>>>>>>>>>>> to >>>>>>>>>>>>>>>> be >>>>>>>>>>>>>>>>>>>>>>>>>> changed (KStream-KTable >>>>>>>>>>>>>>>>>>>>>>>>>> joins is one case). However all of them has >>> similar >>>>>>>>>>>> structure: >>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>>> overload >>>>>>>>>>>>>>>>>>>>>>>>>> the *method* with *methodWithKey*, >>>>>>>>>>>>>>>>>>>>>>>>>> wrap it into the Rich function, send to processor >>>> and >>>>>>>>>>> inside >>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>> processor >>>>>>>>>>>>>>>>>>>>>>>>>> call *init* and *close* methods of the Rich >>>> function. >>>>>>>>>>>>>>>>>>>>>>>>>> As I wrote in KIP, I wanted to demonstrate the >>>>>> overall >>>>>>>>>>> idea >>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>>>>> only >>>>>>>>>>>>>>>>>>>>>>>>>> *ValueMapper* as the same can be applied to all >>>>>>> changes. >>>>>>>>>>>>>>>>>>>>>>>>>> Anyway I will update the KIP. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> - Why do we need `AbstractRichFunction`? >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Instead of overriding the *init(ProcessorContext >>> p)* >>>>>>> and* >>>>>>>>>>>>>>>> close()* >>>>>>>>>>>>>>>>>>>>>>>>> methods >>>>>>>>>>>>>>>>>>>>>>>>>> in every Rich function with empty body like: >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> @Override >>>>>>>>>>>>>>>>>>>>>>>>>> void init(ProcessorContext context) {} >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> @Override >>>>>>>>>>>>>>>>>>>>>>>>>> void close () {} >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> I thought that we can override them once in >>>>>>>>>>>>>>>> *AbstractRichFunction* >>>>>>>>>>>>>>>>>>>>>> and >>>>>>>>>>>>>>>>>>>>>>>>>> extent new Rich functions from >>>>>> *AbstractRichFunction*. >>>>>>>>>>>>>>>>>>>>>>>>>> Basically this can eliminate code copy-paste and >>>> ease >>>>>>> the >>>>>>>>>>>>>>>>>>>>>> maintenance. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> - What about interfaces Initializer, >>> ForeachAction, >>>>>>>>>>> Merger, >>>>>>>>>>>>>>>>>>>>>> Predicate, >>>>>>>>>>>>>>>>>>>>>>>>>>> Reducer? I don't want to say we should/need to >>> add >>>>>> to >>>>>>>>>>> all, >>>>>>>>>>>>>> but >>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>> should >>>>>>>>>>>>>>>>>>>>>>>>>>> discuss all of them and add where it does make >>>> sense >>>>>>>>>>> (e.g., >>>>>>>>>>>>>>>>>>>>>>>>>>> RichForachAction does make sense IMHO) >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Definitely agree. As I said, the same technique >>>>>> applies >>>>>>>>> to >>>>>>>>>>>> all >>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>>>>>>>> interfaces and I didn't want to explode the KIP, >>>> just >>>>>>>>>>> wanted >>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>> give >>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>> overall intuition. >>>>>>>>>>>>>>>>>>>>>>>>>> However, I will update the KIP as I said. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Btw: I like the hierarchy `ValueXX` -- >>>>>> `ValueXXWithKey` >>>>>>>>> -- >>>>>>>>>>>>>>>>>>>>>>> `RichValueXX` >>>>>>>>>>>>>>>>>>>>>>>>>>> in general -- but why can't we do all this with >>>>>>>>>>> interfaces >>>>>>>>>>>>>>>> only? >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Sure we can. However the main intuition is we >>> should >>>>>>> not >>>>>>>>>>>> force >>>>>>>>>>>>>>>>>>>>> users >>>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>> implement *init(ProcessorContext)* and *close()* >>>>>>>>> functions >>>>>>>>>>>>>> every >>>>>>>>>>>>>>>>>>>>> time >>>>>>>>>>>>>>>>>>>>>>>>> they >>>>>>>>>>>>>>>>>>>>>>>>>> use Rich functions. >>>>>>>>>>>>>>>>>>>>>>>>>> If one needs, she can override the respective >>>>>> methods. >>>>>>>>>>>>>> However, >>>>>>>>>>>>>>>> I >>>>>>>>>>>>>>>>>>>>> am >>>>>>>>>>>>>>>>>>>>>>> open >>>>>>>>>>>>>>>>>>>>>>>>>> for discussion. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> I'd rather not see the use of `ProcessorContext` >>>>>>> spread >>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>> further >>>>>>>>>>>>>>>>>>>>>>> than >>>>>>>>>>>>>>>>>>>>>>>>>>> it currently is. So maybe we need another KIP >>> that >>>>>> is >>>>>>>>>>> done >>>>>>>>>>>>>>>> before >>>>>>>>>>>>>>>>>>>>>>> this? >>>>>>>>>>>>>>>>>>>>>>>>>>> Otherwise i think the scope of this KIP is >>> becoming >>>>>>> too >>>>>>>>>>>>>> large. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> That is good point. I wanted to make >>>>>>>>>>>> *init(ProcessorContext)* >>>>>>>>>>>>>>>>>>>>> method >>>>>>>>>>>>>>>>>>>>>>>>>> persistent among the library (which use >>>>>>> ProcessorContext >>>>>>>>>>> as >>>>>>>>>>>> an >>>>>>>>>>>>>>>>>>>>>> input), >>>>>>>>>>>>>>>>>>>>>>>>>> therefore I put *ProcessorContext* as an input. >>>>>>>>>>>>>>>>>>>>>>>>>> So the important question is that (as @dguy and >>>>>> @mjsax >>>>>>>>>>>>>>>> mentioned) >>>>>>>>>>>>>>>>>>>>>>> whether >>>>>>>>>>>>>>>>>>>>>>>>>> continue this KIP without providing users an >>> access >>>>>> to >>>>>>>>>>>>>>>>>>>>>>> *ProcessorContext* >>>>>>>>>>>>>>>>>>>>>>>>>> (change *init (ProcessorContext)* to * init()* ) >>> or >>>>>>>>>>>>>>>>>>>>>>>>>> initiate another KIP before this. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> [1] >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>> http://cr.openjdk.java.net/~mr/se/8/java-se-8-pfd-spec/java- >>>>>>>>>>>>>>>>>>>>>>>>> se-8-jls-pfd-diffs.pdf >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> On Mon, May 15, 2017 at 7:15 PM, Damian Guy < >>>>>>>>>>>>>>>> damian....@gmail.com> >>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> I'd rather not see the use of `ProcessorContext` >>>>>>> spread >>>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>> further >>>>>>>>>>>>>>>>>>>>>>>>> than >>>>>>>>>>>>>>>>>>>>>>>>>>> it currently is. So maybe we need another KIP >>> that >>>>>> is >>>>>>>>>>> done >>>>>>>>>>>>>>>> before >>>>>>>>>>>>>>>>>>>>>>> this? >>>>>>>>>>>>>>>>>>>>>>>>>>> Otherwise i think the scope of this KIP is >>> becoming >>>>>>> too >>>>>>>>>>>>>> large. >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> On Mon, 15 May 2017 at 18:06 Matthias J. Sax < >>>>>>>>>>>>>>>>>>>>> matth...@confluent.io >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> I agree that that `ProcessorContext` interface >>> is >>>>>> too >>>>>>>>>>>> broad >>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>>> general >>>>>>>>>>>>>>>>>>>>>>>>>>>> -- this is even true for transform/process, and >>>>>> it's >>>>>>>>>>> also >>>>>>>>>>>>>>>>>>>>> reflected >>>>>>>>>>>>>>>>>>>>>>> in >>>>>>>>>>>>>>>>>>>>>>>>>>>> the API improvement list we want to do. >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/ >>>> confluence/display/KAFKA/ >>>>>>>>>>>>>>>>>>>>>>>>>>> Kafka+Streams+Discussions >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> So I am wondering, if you question the >>>>>> `RichFunction` >>>>>>>>>>>>>>>> approach in >>>>>>>>>>>>>>>>>>>>>>>>>>>> general? Or if you suggest to either extend the >>>>>> scope >>>>>>>>> of >>>>>>>>>>>>>> this >>>>>>>>>>>>>>>> KIP >>>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>>>> include this---or maybe better, do another KIP >>> for >>>>>> it >>>>>>>>>>> and >>>>>>>>>>>>>>>> delay >>>>>>>>>>>>>>>>>>>>>> this >>>>>>>>>>>>>>>>>>>>>>>>> KIP >>>>>>>>>>>>>>>>>>>>>>>>>>>> until the other one is done? >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> On 5/15/17 2:35 AM, Damian Guy wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP. >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> I'm not convinced on the `RichFunction` >>> approach. >>>>>> Do >>>>>>>>> we >>>>>>>>>>>>>>>> really >>>>>>>>>>>>>>>>>>>>>> want >>>>>>>>>>>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>>>>>>>>> give >>>>>>>>>>>>>>>>>>>>>>>>>>>>> every DSL method access to the >>> `ProcessorContext` >>>>>> ? >>>>>>> It >>>>>>>>>>>> has >>>>>>>>>>>>>> a >>>>>>>>>>>>>>>>>>>>> bunch >>>>>>>>>>>>>>>>>>>>>>> of >>>>>>>>>>>>>>>>>>>>>>>>>>>>> methods on it that seem in-appropriate for some >>>> of >>>>>>> the >>>>>>>>>>>> DSL >>>>>>>>>>>>>>>>>>>>>> methods, >>>>>>>>>>>>>>>>>>>>>>>>>>> i.e, >>>>>>>>>>>>>>>>>>>>>>>>>>>>> `register`, `getStateStore`, `forward`, >>>> `schedule` >>>>>>>>> etc. >>>>>>>>>>>> It >>>>>>>>>>>>>> is >>>>>>>>>>>>>>>>>>>>> far >>>>>>>>>>>>>>>>>>>>>>> too >>>>>>>>>>>>>>>>>>>>>>>>>>>>> broad. I think it would be better to have a >>>>>> narrower >>>>>>>>>>>>>>>> interface >>>>>>>>>>>>>>>>>>>>>> like >>>>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>> `RecordContext` - remembering it is easier to >>>> add >>>>>>>>>>>>>>>>>>>>>>> methods/interfaces >>>>>>>>>>>>>>>>>>>>>>>>>>>> later >>>>>>>>>>>>>>>>>>>>>>>>>>>>> than to remove them >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Sat, 13 May 2017 at 22:26 Matthias J. Sax < >>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I am not an expert on Lambdas. Can you >>> elaborate >>>>>> a >>>>>>>>>>>> little >>>>>>>>>>>>>>>> bit? >>>>>>>>>>>>>>>>>>>>> I >>>>>>>>>>>>>>>>>>>>>>>>>>> cannot >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> follow the explanation in the KIP to see what >>>> the >>>>>>>>>>>> problem >>>>>>>>>>>>>>>> is. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For updating the KIP title I don't know -- >>> guess >>>>>>> it's >>>>>>>>>>> up >>>>>>>>>>>>>> to >>>>>>>>>>>>>>>>>>>>> you. >>>>>>>>>>>>>>>>>>>>>>>>>>> Maybe a >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> committer can comment on this? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Further comments: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - The KIP get a little hard to read -- can >>> you >>>>>>> maybe >>>>>>>>>>>>>>>> reformat >>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>> wiki >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> page a little bit? I think using `CodeBlock` >>>>>> would >>>>>>>>>>> help. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - What about KStream-KTable joins? You don't >>>>>> have >>>>>>>>>>>>>> overlaods >>>>>>>>>>>>>>>>>>>>>> added >>>>>>>>>>>>>>>>>>>>>>>>> for >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> them. Why? (Even if I still hope that we don't >>>>>> need >>>>>>>>> to >>>>>>>>>>>> add >>>>>>>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>>> new >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> overloads) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - Why do we need `AbstractRichFunction`? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> - What about interfaces Initializer, >>>>>>> ForeachAction, >>>>>>>>>>>>>> Merger, >>>>>>>>>>>>>>>>>>>>>>>>>>> Predicate, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Reducer? I don't want to say we should/need to >>>>>> add >>>>>>> to >>>>>>>>>>>> all, >>>>>>>>>>>>>>>> but >>>>>>>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>>>>> should >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> discuss all of them and add where it does make >>>>>>> sense >>>>>>>>>>>>>> (e.g., >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RichForachAction does make sense IMHO) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Btw: I like the hierarchy `ValueXX` -- >>>>>>>>>>> `ValueXXWithKey` >>>>>>>>>>>> -- >>>>>>>>>>>>>>>>>>>>>>>>>>> `RichValueXX` >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> in general -- but why can't we do all this >>> with >>>>>>>>>>>> interfaces >>>>>>>>>>>>>>>>>>>>> only? >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 5/11/17 7:00 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for your comments. I think we cannot >>>>>> extend >>>>>>>>>>> the >>>>>>>>>>>>>> two >>>>>>>>>>>>>>>>>>>>>>>>> interfaces >>>>>>>>>>>>>>>>>>>>>>>>>>>> if >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> want to keep lambdas. I updated the KIP [1]. >>>>>>> Maybe I >>>>>>>>>>>>>> should >>>>>>>>>>>>>>>>>>>>>> change >>>>>>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> title, because now we are not limiting the >>> KIP >>>>>> to >>>>>>>>>>> only >>>>>>>>>>>>>>>>>>>>>>> ValueMapper, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ValueTransformer and ValueJoiner. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Please feel free to comment. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> [1] >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/ >>>>>>> confluence/display/KAFKA/KIP- >>>>>>>>>>>>>>>>>>>>>>>>>>> 149%3A+Enabling+key+access+in+ >>> ValueTransformer%2C+ >>>>>>>>>>>>>>>>>>>>>>>>>>> ValueMapper%2C+and+ValueJoiner >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, May 9, 2017 at 7:36 PM Matthias J. >>> Sax >>>> < >>>>>>>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> If `ValueMapperWithKey` extends >>> `ValueMapper` >>>>>> we >>>>>>>>>>> don't >>>>>>>>>>>>>>>> need >>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>> new >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> overlaod. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> And yes, we need to do one check -- but this >>>>>>>>> happens >>>>>>>>>>>>>> when >>>>>>>>>>>>>>>>>>>>>>> building >>>>>>>>>>>>>>>>>>>>>>>>>>> the >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> topology. At runtime (I mean after >>>>>>>>>>> KafkaStream#start() >>>>>>>>>>>>>> we >>>>>>>>>>>>>>>>>>>>> don't >>>>>>>>>>>>>>>>>>>>>>>>> need >>>>>>>>>>>>>>>>>>>>>>>>>>>> any >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> check as we will always use >>>>>> `ValueMapperWithKey`) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 5/9/17 2:55 AM, Jeyhun Karimov wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for feedback. >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Then we need to overload method >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> <VR> KStream<K, VR> >>> mapValues(ValueMapper<? >>>>>>>>> super >>>>>>>>>>>> V, >>>>>>>>>>>>>> ? >>>>>>>>>>>>>>>>>>>>>> extends >>>>>>>>>>>>>>>>>>>>>>>>>>> VR> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> mapper); >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> with >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> <VR> KStream<K, VR> >>>>>>>>>>> mapValues(ValueMapperWithKey<? >>>>>>>>>>>>>>>> super >>>>>>>>>>>>>>>>>>>>> V, >>>>>>>>>>>>>>>>>>>>>> ? >>>>>>>>>>>>>>>>>>>>>>>>>>>> extends >>>>>>>>>>>>>>>>>>> >>> >> >> >> >
signature.asc
Description: OpenPGP digital signature
