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