Thanks. I understand what you are saying, but I don't agree that > but also we need a commit() method
I would just not provide `commit()` at DSL level and close the
corresponding Jira as "not a problem" or similar.
-Matthias
On 10/27/17 3:42 PM, Jeyhun Karimov wrote:
> Hi Matthias,
>
> Thanks for your comments. I agree that this is not the best way to do. A
> bit of history behind this design.
>
> Prior doing this, I tried to provide ProcessorContext itself as an argument
> in Rich interfaces. However, we dont want to give users that flexibility
> and “power”. Moreover, ProcessorContext contains processor level
> information and not Record level info. The only thing we need ij
> ProcessorContext is commit() method.
>
> So, as far as I understood, we need recor context (offset, timestamp and
> etc) but also we need a commit() method ( we dont want to provide
> ProcessorContext as a parameter so users can use ProcessorContext.commit()
> ).
>
> As a result, I thought to “propagate” commit() call from RecordContext to
> ProcessorContext() .
>
>
> If there is a misunderstanding in motvation/discussion of KIP/included
> jiras please let me know.
>
>
> Cheers,
> Jeyhun
>
>
> On Fri 27. Oct 2017 at 12:39, Matthias J. Sax <matth...@confluent.io> wrote:
>
>> I am personally still not convinced, that we should add `commit()` at all.
>>
>> @Guozhang: you created the original Jira. Can you elaborate a little
>> bit? Isn't requesting commits a low level API that should not be exposed
>> in the DSL? Just want to understand the motivation better. Why would
>> anybody that uses the DSL ever want to request a commit? To me,
>> requesting commits is useful if you manipulated state explicitly, ie,
>> via Processor API.
>>
>> Also, for the solution: it seem rather unnatural to me, that we add
>> `commit()` to `RecordContext` -- from my understanding, `RecordContext`
>> is an helper object that provide access to record meta data. Requesting
>> a commit is something quite different. Additionally, a commit does not
>> commit a specific record but a `RecrodContext` is for a specific record.
>>
>> To me, this does not seem to be a sound API design if we follow this path.
>>
>>
>> -Matthias
>>
>>
>>
>> On 10/26/17 10:41 PM, Jeyhun Karimov wrote:
>>> Hi,
>>>
>>> Thanks for your suggestions.
>>>
>>> I have some comments, to make sure that there is no misunderstanding.
>>>
>>>
>>> 1. Maybe we can deprecate the `commit()` in ProcessorContext, to enforce
>>>> user to consolidate this call as
>>>> "processorContext.recordContext().commit()". And internal implementation
>>>> of
>>>> `ProcessorContext.commit()` in `ProcessorContextImpl` is also changed to
>>>> this call.
>>>
>>>
>>> - I think we should not deprecate `ProcessorContext.commit()`. The main
>>> intuition that we introduce `commit()` in `RecordContext` is that,
>>> `RecordContext` is the one which is provided in Rich interfaces. So if
>> user
>>> wants to commit, then there should be some method inside `RecordContext`
>> to
>>> do so. Internally, `RecordContext.commit()` calls
>>> `ProcessorContext.commit()` (see the last code snippet in KIP-159):
>>>
>>> @Override
>>> public void process(final K1 key, final V1 value) {
>>>
>>> recordContext = new RecordContext() { //
>>> recordContext initialization is added in this KIP
>>> @Override
>>> public void commit() {
>>> context().commit();
>>> }
>>>
>>> @Override
>>> public long offset() {
>>> return context().recordContext().offset();
>>> }
>>>
>>> @Override
>>> public long timestamp() {
>>> return context().recordContext().timestamp();
>>> }
>>>
>>> @Override
>>> public String topic() {
>>> return context().recordContext().topic();
>>> }
>>>
>>> @Override
>>> public int partition() {
>>> return context().recordContext().partition();
>>> }
>>> };
>>>
>>>
>>> So, we cannot deprecate `ProcessorContext.commit()` in this case IMO.
>>>
>>>
>>> 2. Add the `task` reference to the impl class, `ProcessorRecordContext`,
>> so
>>>> that it can implement the commit call itself.
>>>
>>>
>>> - Actually, I don't think that we need `commit()` in
>>> `ProcessorRecordContext`. The main intuition is to "transfer"
>>> `ProcessorContext.commit()` call to Rich interfaces, to support
>>> user-specific committing.
>>> To do so, we introduce `commit()` method in `RecordContext()` just only
>> to
>>> call ProcessorContext.commit() inside. (see the above code snippet)
>>> So, in Rich interfaces, we are not dealing with `ProcessorRecordContext`
>>> at all, and we leave all its methods as it is.
>>> In this KIP, we made `RecordContext` to be the parent class of
>>> `ProcessorRecordContext`, just because of they share quite amount of
>>> methods and it is logical to enable inheritance between those two.
>>>
>>> 3. In the wiki page, the statement that "However, call to a commit()
>> method,
>>>> is valid only within RecordContext interface (at least for now), we
>> throw
>>>> an exception in ProcessorRecordContext.commit()." and the code snippet
>>>> below would need to be updated as well.
>>>
>>>
>>> - I think above explanation covers this as well.
>>>
>>>
>>> I want to gain some speed to this KIP, as it has gone though many changes
>>> based on user/developer needs, both in
>> documentation-/implementation-wise.
>>>
>>>
>>> Cheers,
>>> Jeyhun
>>>
>>>
>>>
>>> On Tue, Oct 24, 2017 at 1:41 AM Guozhang Wang <wangg...@gmail.com>
>> wrote:
>>>
>>>> Thanks for the information Jeyhun. I had also forgot about KAFKA-3907
>> with
>>>> this KIP..
>>>>
>>>> Thinking a bit more, I'm now inclined to go with what we agreed before,
>> to
>>>> add the commit() call to `RecordContext`. A few minor tweaks on its
>>>> implementation:
>>>>
>>>> 1. Maybe we can deprecate the `commit()` in ProcessorContext, to enforce
>>>> user to consolidate this call as
>>>> "processorContext.recordContext().commit()". And internal implementation
>>>> of
>>>> `ProcessorContext.commit()` in `ProcessorContextImpl` is also changed to
>>>> this call.
>>>>
>>>> 2. Add the `task` reference to the impl class,
>> `ProcessorRecordContext`, so
>>>> that it can implement the commit call itself.
>>>>
>>>> 3. In the wiki page, the statement that "However, call to a commit()
>>>> method,
>>>> is valid only within RecordContext interface (at least for now), we
>> throw
>>>> an exception in ProcessorRecordContext.commit()." and the code snippet
>>>> below would need to be updated as well.
>>>>
>>>>
>>>> Guozhang
>>>>
>>>>
>>>>
>>>> On Mon, Oct 23, 2017 at 1:40 PM, Matthias J. Sax <matth...@confluent.io
>>>
>>>> wrote:
>>>>
>>>>> Fair point. This is a long discussion and I totally forgot that we
>>>>> discussed this.
>>>>>
>>>>> Seems I changed my opinion about including KAFKA-3907...
>>>>>
>>>>> Happy to hear what others think.
>>>>>
>>>>>
>>>>> -Matthias
>>>>>
>>>>> On 10/23/17 1:20 PM, Jeyhun Karimov wrote:
>>>>>> Hi Matthias,
>>>>>>
>>>>>> It is probably my bad, the discussion was a bit long in this thread. I
>>>>>> proposed the related issue in the related KIP discuss thread [1] and
>>>> got
>>>>> an
>>>>>> approval [2,3].
>>>>>> Maybe I misunderstood.
>>>>>>
>>>>>> [1]
>>>>>> http://search-hadoop.com/m/Kafka/uyzND19Asmg1GKKXT1?subj=
>>>>> Re+DISCUSS+KIP+159+Introducing+Rich+functions+to+Streams
>>>>>> [2]
>>>>>> http://search-hadoop.com/m/Kafka/uyzND1kpct22GKKXT1?subj=
>>>>> Re+DISCUSS+KIP+159+Introducing+Rich+functions+to+Streams
>>>>>> [3]
>>>>>> http://search-hadoop.com/m/Kafka/uyzND1G6TGIGKKXT1?subj=
>>>>> Re+DISCUSS+KIP+159+Introducing+Rich+functions+to+Streams
>>>>>>
>>>>>>
>>>>>> On Mon, Oct 23, 2017 at 8:44 PM Matthias J. Sax <
>> matth...@confluent.io
>>>>>
>>>>>> wrote:
>>>>>>
>>>>>>> Interesting.
>>>>>>>
>>>>>>> I thought that https://issues.apache.org/jira/browse/KAFKA-4125 is
>>>> the
>>>>>>> main motivation for this KIP :)
>>>>>>>
>>>>>>> I also think, that we should not expose the full ProcessorContext at
>>>> DSL
>>>>>>> level.
>>>>>>>
>>>>>>> Thus, overall I am not even sure if we should fix KAFKA-3907 at all.
>>>>>>> Manual commits are something DSL users should not worry about -- and
>>>> if
>>>>>>> one really needs this, an advanced user can still insert a dummy
>>>>>>> `transform` to request a commit from there.
>>>>>>>
>>>>>>> -Matthias
>>>>>>>
>>>>>>>
>>>>>>> On 10/18/17 5:39 AM, Jeyhun Karimov wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> The main intuition is to solve [1], which is part of this KIP.
>>>>>>>> I agree with you that this might not seem semantically correct as we
>>>>> are
>>>>>>>> not committing record state.
>>>>>>>> Alternatively, we can remove commit() from RecordContext and add
>>>>>>>> ProcessorContext (which has commit() method) as an extra argument to
>>>>> Rich
>>>>>>>> methods:
>>>>>>>>
>>>>>>>> instead of
>>>>>>>> public interface RichValueMapper<V, VR, K> {
>>>>>>>> VR apply(final V value,
>>>>>>>> final K key,
>>>>>>>> final RecordContext recordContext);
>>>>>>>> }
>>>>>>>>
>>>>>>>> we can adopt
>>>>>>>>
>>>>>>>> public interface RichValueMapper<V, VR, K> {
>>>>>>>> VR apply(final V value,
>>>>>>>> final K key,
>>>>>>>> final RecordContext recordContext,
>>>>>>>> final ProcessorContext processorContext);
>>>>>>>> }
>>>>>>>>
>>>>>>>>
>>>>>>>> However, in this case, a user can get confused as ProcessorContext
>>>> and
>>>>>>>> RecordContext share some methods with the same name.
>>>>>>>>
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Jeyhun
>>>>>>>>
>>>>>>>>
>>>>>>>> [1] https://issues.apache.org/jira/browse/KAFKA-3907
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Oct 17, 2017 at 3:19 AM Guozhang Wang <wangg...@gmail.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Regarding #6 above, I'm still not clear why we would need
>> `commit()`
>>>>> in
>>>>>>>>> both ProcessorContext and RecordContext, could you elaborate a bit
>>>>> more?
>>>>>>>>>
>>>>>>>>> To me `commit()` is really a processor context not a record context
>>>>>>>>> logically: when you call that function, it means we would commit
>> the
>>>>>>> state
>>>>>>>>> of the whole task up to this processed record, not only that single
>>>>>>> record
>>>>>>>>> itself.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Guozhang
>>>>>>>>>
>>>>>>>>> On Mon, Oct 16, 2017 at 9:19 AM, Jeyhun Karimov <
>>>> je.kari...@gmail.com
>>>>>>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Thanks for the feedback.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 0. RichInitializer definition seems missing.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - Fixed.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I'd suggest moving the key parameter in the RichValueXX and
>>>>>>> RichReducer
>>>>>>>>>>> after the value parameters, as well as in the templates; e.g.
>>>>>>>>>>> public interface RichValueJoiner<V1, V2, VR, K> {
>>>>>>>>>>> VR apply(final V1 value1, final V2 value2, final K key, final
>>>>>>>>>>> RecordContext
>>>>>>>>>>> recordContext);
>>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - Fixed.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2. Some of the listed functions are not necessary since their
>>>> pairing
>>>>>>>>> APIs
>>>>>>>>>>> are being deprecated in 1.0 already:
>>>>>>>>>>> <KR> KGroupedStream<KR, V> groupBy(final RichKeyValueMapper<?
>>>> super
>>>>> K,
>>>>>>>>> ?
>>>>>>>>>>> super V, KR> selector,
>>>>>>>>>>> final Serde<KR> keySerde,
>>>>>>>>>>> final Serde<V> valSerde);
>>>>>>>>>>> <VT, VR> KStream<K, VR> leftJoin(final KTable<K, VT> table,
>>>>>>>>>>> final RichValueJoiner<? super K,
>>>> ?
>>>>>>>>> super
>>>>>>>>>>> V,
>>>>>>>>>>> ? super VT, ? extends VR> joiner,
>>>>>>>>>>> final Serde<K> keySerde,
>>>>>>>>>>> final Serde<V> valSerde);
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -Fixed
>>>>>>>>>>
>>>>>>>>>> 3. For a few functions where we are adding three APIs for a combo
>>>> of
>>>>>>> both
>>>>>>>>>>> mapper / joiner, or both initializer / aggregator, or adder /
>>>>>>>>> subtractor,
>>>>>>>>>>> I'm wondering if we can just keep one that use "rich" functions
>>>> for
>>>>>>>>> both;
>>>>>>>>>>> so that we can have less overloads and let users who only want to
>>>>>>>>> access
>>>>>>>>>>> one of them to just use dummy parameter declarations. For
>> example:
>>>>>>>>>>>
>>>>>>>>>>> <GK, GV, RV> KStream<K, RV> join(final GlobalKTable<GK, GV>
>>>>>>>>> globalKTable,
>>>>>>>>>>> final RichKeyValueMapper<? super
>>>>> K, ?
>>>>>>>>>>> super
>>>>>>>>>>> V, ? extends GK> keyValueMapper,
>>>>>>>>>>> final RichValueJoiner<? super K,
>>>> ?
>>>>>>>>> super
>>>>>>>>>>> V,
>>>>>>>>>>> ? super GV, ? extends RV> joiner);
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -Agreed. Fixed.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 4. For TimeWindowedKStream, I'm wondering why we do not make its
>>>>>>>>>>> Initializer also "rich" functions? I.e.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> - It was a typo. Fixed.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 5. We need to move "RecordContext" from o.a.k.processor.internals
>>>> to
>>>>>>>>>>> o.a.k.processor.
>>>>>>>>>>>
>>>>>>>>>>> 6. I'm not clear why we want to move `commit()` from
>>>>> ProcessorContext
>>>>>>>>> to
>>>>>>>>>>> RecordContext?
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -
>>>>>>>>>> Because it makes sense logically and to reduce code maintenance
>>>>> (both
>>>>>>>>>> interfaces have offset() timestamp() topic() partition()
>>>> methods), I
>>>>>>>>>> inherit ProcessorContext from RecordContext.
>>>>>>>>>> Since we need commit() method both in ProcessorContext and in
>>>>>>>>> RecordContext
>>>>>>>>>> I move commit() method to parent class (RecordContext).
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> Jeyhun
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Wed, Oct 11, 2017 at 12:59 AM, Guozhang Wang <
>>>> wangg...@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> Jeyhun,
>>>>>>>>>>>
>>>>>>>>>>> Thanks for the updated KIP, here are my comments.
>>>>>>>>>>>
>>>>>>>>>>> 0. RichInitializer definition seems missing.
>>>>>>>>>>>
>>>>>>>>>>> 1. I'd suggest moving the key parameter in the RichValueXX and
>>>>>>>>>> RichReducer
>>>>>>>>>>> after the value parameters, as well as in the templates; e.g.
>>>>>>>>>>>
>>>>>>>>>>> public interface RichValueJoiner<V1, V2, VR, K> {
>>>>>>>>>>> VR apply(final V1 value1, final V2 value2, final K key, final
>>>>>>>>>>> RecordContext
>>>>>>>>>>> recordContext);
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> My motivation is that for lambda expression in J8, users that
>>>> would
>>>>>>> not
>>>>>>>>>>> care about the key but only the context, or vice versa, is likely
>>>> to
>>>>>>>>>> write
>>>>>>>>>>> it as (value1, value2, dummy, context) -> ... than putting the
>>>> dummy
>>>>>>> at
>>>>>>>>>> the
>>>>>>>>>>> beginning of the parameter list. Generally speaking we'd like to
>>>>> make
>>>>>>>>> all
>>>>>>>>>>> the "necessary" parameters prior to optional ones.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 2. Some of the listed functions are not necessary since their
>>>>> pairing
>>>>>>>>>> APIs
>>>>>>>>>>> are being deprecated in 1.0 already:
>>>>>>>>>>>
>>>>>>>>>>> <KR> KGroupedStream<KR, V> groupBy(final RichKeyValueMapper<?
>>>> super
>>>>> K,
>>>>>>>>> ?
>>>>>>>>>>> super V, KR> selector,
>>>>>>>>>>> final Serde<KR> keySerde,
>>>>>>>>>>> final Serde<V> valSerde);
>>>>>>>>>>>
>>>>>>>>>>> <VT, VR> KStream<K, VR> leftJoin(final KTable<K, VT> table,
>>>>>>>>>>> final RichValueJoiner<? super K,
>>>> ?
>>>>>>>>> super
>>>>>>>>>>> V,
>>>>>>>>>>> ? super VT, ? extends VR> joiner,
>>>>>>>>>>> final Serde<K> keySerde,
>>>>>>>>>>> final Serde<V> valSerde);
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 3. For a few functions where we are adding three APIs for a combo
>>>> of
>>>>>>>>> both
>>>>>>>>>>> mapper / joiner, or both initializer / aggregator, or adder /
>>>>>>>>> subtractor,
>>>>>>>>>>> I'm wondering if we can just keep one that use "rich" functions
>>>> for
>>>>>>>>> both;
>>>>>>>>>>> so that we can have less overloads and let users who only want to
>>>>>>>>> access
>>>>>>>>>>> one of them to just use dummy parameter declarations. For
>> example:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> <GK, GV, RV> KStream<K, RV> join(final GlobalKTable<GK, GV>
>>>>>>>>> globalKTable,
>>>>>>>>>>> final RichKeyValueMapper<? super
>>>>> K, ?
>>>>>>>>>>> super
>>>>>>>>>>> V, ? extends GK> keyValueMapper,
>>>>>>>>>>> final RichValueJoiner<? super K,
>>>> ?
>>>>>>>>> super
>>>>>>>>>>> V,
>>>>>>>>>>> ? super GV, ? extends RV> joiner);
>>>>>>>>>>>
>>>>>>>>>>> <VR> KTable<K, VR> aggregate(final RichInitializer<K, VR>
>>>>> initializer,
>>>>>>>>>>> final RichAggregator<? super K, ?
>>>> super
>>>>>>> V,
>>>>>>>>>> VR>
>>>>>>>>>>> aggregator,
>>>>>>>>>>> final Materialized<K, VR,
>>>>>>>>>> KeyValueStore<Bytes,
>>>>>>>>>>> byte[]>> materialized);
>>>>>>>>>>>
>>>>>>>>>>> Similarly for KGroupedTable, a bunch of aggregate() are
>> deprecated
>>>>> so
>>>>>>>>> we
>>>>>>>>>> do
>>>>>>>>>>> not need to add its rich functions any more.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 4. For TimeWindowedKStream, I'm wondering why we do not make its
>>>>>>>>>>> Initializer also "rich" functions? I.e.
>>>>>>>>>>>
>>>>>>>>>>> <VR> KTable<Windowed<K>, VR> aggregate(final RichInitializer<VR,
>>>> K>
>>>>>>>>>>> initializer,
>>>>>>>>>>> final RichAggregator<?
>>>> super
>>>>> K,
>>>>>>>>> ?
>>>>>>>>>>> super V, VR> aggregator);
>>>>>>>>>>> <VR> KTable<Windowed<K>, VR> aggregate(final RichInitializer<VR,
>>>> K>
>>>>>>>>>>> initializer,
>>>>>>>>>>> final RichAggregator<?
>>>> super
>>>>> K,
>>>>>>>>> ?
>>>>>>>>>>> super V, VR> aggregator,
>>>>>>>>>>> final Materialized<K, VR,
>>>>>>>>>>> WindowStore<Bytes, byte[]>> materialized);
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> 5. We need to move "RecordContext" from o.a.k.processor.internals
>>>> to
>>>>>>>>>>> o.a.k.processor.
>>>>>>>>>>>
>>>>>>>>>>> 6. I'm not clear why we want to move `commit()` from
>>>>> ProcessorContext
>>>>>>>>> to
>>>>>>>>>>> RecordContext? Conceptually I think it would better staying in
>> the
>>>>>>>>>>> ProcessorContext. Do you find this not doable in the internal
>>>>>>>>>>> implementations?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Guozhang
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Fri, Sep 22, 2017 at 1:09 PM, Ted Yu <yuzhih...@gmail.com>
>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> recordContext = new RecordContext() { //
>>>>>>>>> recordContext
>>>>>>>>>>>> initialization is added in this KIP
>>>>>>>>>>>>
>>>>>>>>>>>> This code snippet seems to be standard - would it make sense to
>>>>> pull
>>>>>>>>> it
>>>>>>>>>>>> into a (sample) RecordContext implementation ?
>>>>>>>>>>>>
>>>>>>>>>>>> Cheers
>>>>>>>>>>>>
>>>>>>>>>>>> On Fri, Sep 22, 2017 at 12:14 PM, Jeyhun Karimov <
>>>>>>>>> je.kari...@gmail.com
>>>>>>>>>>>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Ted,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for your comments. I added a couple of comments in KIP
>> to
>>>>>>>>>>> clarify
>>>>>>>>>>>>> some points.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> bq. provides a hybrd solution
>>>>>>>>>>>>>> Typo in hybrid.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> - My bad. Thanks for the correction.
>>>>>>>>>>>>>
>>>>>>>>>>>>> It would be nice if you can name some Value operator as
>>>> examples.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>> - I added the corresponding interface names to KIP.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> <VR> KTable<K, VR> aggregate(final Initializer<VR> initializer,
>>>>>>>>>>>>>> final Aggregator<? super K, ?
>>>> super
>>>>>>>>> V,
>>>>>>>>>>> VR>
>>>>>>>>>>>>>> adder,
>>>>>>>>>>>>>> The adder doesn't need to be RichAggregator ?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> - Exactly. However, there are 2 Aggregator-type arguments in
>> the
>>>>>>>>>>> related
>>>>>>>>>>>>> method. So, I had to overload all possible their Rich
>>>>> counterparts:
>>>>>>>>>>>>>
>>>>>>>>>>>>> // adder with non-rich, subtrctor is rich
>>>>>>>>>>>>> <VR> KTable<K, VR> aggregate(final Initializer<VR> initializer,
>>>>>>>>>>>>> final Aggregator<? super K, ?
>> super
>>>>> V,
>>>>>>>>>> VR>
>>>>>>>>>>>>> adder,
>>>>>>>>>>>>> final RichAggregator<? super K, ?
>>>>>>>>> super
>>>>>>>>>> V,
>>>>>>>>>>>> VR>
>>>>>>>>>>>>> subtractor,
>>>>>>>>>>>>> final Materialized<K, VR,
>>>>>>>>>>>> KeyValueStore<Bytes,
>>>>>>>>>>>>> byte[]>> materialized);
>>>>>>>>>>>>>
>>>>>>>>>>>>> // adder withrich, subtrctor is non-rich
>>>>>>>>>>>>> <VR> KTable<K, VR> aggregate(final Initializer<VR> initializer,
>>>>>>>>>>>>> final RichAggregator<? super K, ?
>>>>>>>>> super
>>>>>>>>>> V,
>>>>>>>>>>>> VR>
>>>>>>>>>>>>> adder,
>>>>>>>>>>>>> final Aggregator<? super K, ?
>> super
>>>>> V,
>>>>>>>>>> VR>
>>>>>>>>>>>>> subtractor,
>>>>>>>>>>>>> final Materialized<K, VR,
>>>>>>>>>>>> KeyValueStore<Bytes,
>>>>>>>>>>>>> byte[]>> materialized);
>>>>>>>>>>>>>
>>>>>>>>>>>>> // both adder and subtractor are rich
>>>>>>>>>>>>> <VR> KTable<K, VR> aggregate(final Initializer<VR> initializer,
>>>>>>>>>>>>> final RichAggregator<? super K, ?
>>>>>>>>> super
>>>>>>>>>> V,
>>>>>>>>>>>> VR>
>>>>>>>>>>>>> adder,
>>>>>>>>>>>>> final RichAggregator<? super K, ?
>>>>>>>>> super
>>>>>>>>>> V,
>>>>>>>>>>>> VR>
>>>>>>>>>>>>> subtractor,
>>>>>>>>>>>>> final Materialized<K, VR,
>>>>>>>>>>>> KeyValueStore<Bytes,
>>>>>>>>>>>>> byte[]>> materialized);
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Can you explain a bit about the above implementation ?
>>>>>>>>>>>>>> void commit () {
>>>>>>>>>>>>>> throw new UnsupportedOperationException("commit() is not
>>>>>>>>>>>> supported
>>>>>>>>>>>>> in
>>>>>>>>>>>>>> this context");
>>>>>>>>>>>>>> Is the exception going to be replaced with real code in the PR
>>>> ?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> - I added some comments both inside and outside the code
>>>> snippets
>>>>>>>>> in
>>>>>>>>>>> KIP.
>>>>>>>>>>>>> Specifically, for the code snippet above, we add *commit()*
>>>> method
>>>>>>>>> to
>>>>>>>>>>>>> *RecordContext* interface.
>>>>>>>>>>>>> However, we want *commit()* method to be used only for
>>>>>>>>>> *RecordContext*
>>>>>>>>>>>>> instances (at least for now), so we add
>>>>>>>>> UnsupportedOperationException
>>>>>>>>>>> in
>>>>>>>>>>>>> all classes/interfaces that extend/implement *RecordContext.*
>>>>>>>>>>>>> In general, 1) we make RecordContext publicly available within
>>>>>>>>>>>>> ProcessorContext, 2) initialize its instance within all
>>>> required
>>>>>>>>>>>>> Processors and 3) pass it as an argument to the related Rich
>>>>>>>>>> interfaces
>>>>>>>>>>>>> inside Processors.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Fri, Sep 22, 2017 at 6:44 PM Ted Yu <yuzhih...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> bq. provides a hybrd solution
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Typo in hybrid.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> bq. accessing read-only keys within XXXValues operators
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It would be nice if you can name some Value operator as
>>>> examples.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> <VR> KTable<K, VR> aggregate(final Initializer<VR>
>> initializer,
>>>>>>>>>>>>>> final Aggregator<? super K, ?
>>>> super
>>>>>>>>> V,
>>>>>>>>>>> VR>
>>>>>>>>>>>>>> adder,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The adder doesn't need to be RichAggregator ?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> public RecordContext recordContext() {
>>>>>>>>>>>>>> return this.recordContext();
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Can you explain a bit about the above implementation ?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> void commit () {
>>>>>>>>>>>>>> throw new UnsupportedOperationException("commit() is not
>>>>>>>>>>>> supported
>>>>>>>>>>>>> in
>>>>>>>>>>>>>> this context");
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is the exception going to be replaced with real code in the PR
>>>> ?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Cheers
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Fri, Sep 22, 2017 at 9:28 AM, Jeyhun Karimov <
>>>>>>>>>>> je.kari...@gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Dear community,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I updated the related KIP [1]. Please feel free to comment.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>>>>>>>>>>>>>> 159%3A+Introducing+Rich+functions+to+Streams
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Fri, Sep 22, 2017 at 12:20 AM Jeyhun Karimov <
>>>>>>>>>>>> je.kari...@gmail.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi Damian,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Thanks for the update. I working on it and will provide an
>>>>>>>>>> update
>>>>>>>>>>>>> soon.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Thu, Sep 21, 2017 at 4:50 PM Damian Guy <
>>>>>>>>>> damian....@gmail.com
>>>>>>>>>>>>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi Jeyhun,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> All KIP-182 API PRs have now been merged. So you can
>>>>>>>>> consider
>>>>>>>>>> it
>>>>>>>>>>>> as
>>>>>>>>>>>>>>>>> stable.
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>> Damian
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> On Thu, 21 Sep 2017 at 15:23 Jeyhun Karimov <
>>>>>>>>>>> je.kari...@gmail.com
>>>>>>>>>>>>>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Thanks a lot for your comments. For the single interface
>>>>>>>>>>>> (RichXXX
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>> XXXWithKey) solution, I have already submitted a PR but
>>>>>>>>>>> probably
>>>>>>>>>>>>> it
>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>> outdated (when the KIP first proposed), I need to revisit
>>>>>>>>>> that
>>>>>>>>>>>>> one.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> @Guozhang, from our (offline) discussion, I understood
>>>>>>>>> that
>>>>>>>>>> we
>>>>>>>>>>>> may
>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>> make
>>>>>>>>>>>>>>>>>> it merge this KIP into the upcoming release, as KIP-159 is
>>>>>>>>>> not
>>>>>>>>>>>>> voted
>>>>>>>>>>>>>>> yet
>>>>>>>>>>>>>>>>>> (because we want both KIP-149 and KIP-159 to be as an
>>>>>>>>>> "atomic"
>>>>>>>>>>>>>> merge).
>>>>>>>>>>>>>>>>> So
>>>>>>>>>>>>>>>>>> I decided to wait until KIP-182 gets stable (there are
>>>>>>>>> some
>>>>>>>>>>>> minor
>>>>>>>>>>>>>>>>> updates
>>>>>>>>>>>>>>>>>> AFAIK) and update the KIP accordingly. Please correct me
>>>>>>>>> if
>>>>>>>>>> I
>>>>>>>>>>> am
>>>>>>>>>>>>>> wrong
>>>>>>>>>>>>>>>>> or I
>>>>>>>>>>>>>>>>>> misunderstood.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> On Thu, Sep 21, 2017 at 4:11 PM Damian Guy <
>>>>>>>>>>>> damian....@gmail.com>
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> +1
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> On Thu, 21 Sep 2017 at 13:46 Guozhang Wang <
>>>>>>>>>>>> wangg...@gmail.com>
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> +1 for me as well for collapsing.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Jeyhun, could you update the wiki accordingly to show
>>>>>>>>>>> what's
>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> final
>>>>>>>>>>>>>>>>>>>> updates post KIP-182 that needs to be done in KIP-159
>>>>>>>>>>>>> including
>>>>>>>>>>>>>>>>>> KIP-149?
>>>>>>>>>>>>>>>>>>>> The child page I made is just a suggestion, but you
>>>>>>>>>> would
>>>>>>>>>>>>> still
>>>>>>>>>>>>>>>>> need to
>>>>>>>>>>>>>>>>>>>> update your proposal for people to comment and vote
>>>>>>>>> on.
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> Guozhang
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>> On Thu, Sep 14, 2017 at 10:37 PM, Ted Yu <
>>>>>>>>>>>> yuzhih...@gmail.com
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> +1
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> One interface is cleaner.
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> On Thu, Sep 14, 2017 at 7:26 AM, Bill Bejeck <
>>>>>>>>>>>>>> bbej...@gmail.com
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> +1 for me on collapsing the RichXXXX and
>>>>>>>>>>>> ValueXXXXWithKey
>>>>>>>>>>>>>>>>>> interfaces
>>>>>>>>>>>>>>>>>>>>> into 1
>>>>>>>>>>>>>>>>>>>>>> interface.
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>> Bill
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> On Wed, Sep 13, 2017 at 11:31 AM, Jeyhun Karimov <
>>>>>>>>>>>>>>>>>>> je.kari...@gmail.com
>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Hi Damian,
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Thanks for your feedback. Actually, this (what
>>>>>>>>> you
>>>>>>>>>>>>>> propose)
>>>>>>>>>>>>>>>>> was
>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>> first
>>>>>>>>>>>>>>>>>>>>>>> idea of KIP-149. Then we decided to divide it
>>>>>>>>> into
>>>>>>>>>>> two
>>>>>>>>>>>>>>> KIPs. I
>>>>>>>>>>>>>>>>>> also
>>>>>>>>>>>>>>>>>>>>>>> expressed my opinion that keeping the two
>>>>>>>>>> interfaces
>>>>>>>>>>>>> (Rich
>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>> withKey)
>>>>>>>>>>>>>>>>>>>>>>> separate would add more overloads. So, email
>>>>>>>>>>>> discussion
>>>>>>>>>>>>>>>>> resulted
>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>>>>>>> would not be a problem.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Our initial idea was similar to :
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> public abstract class RichValueMapper<K, V, VR>
>>>>>>>>>>>>>> implements
>>>>>>>>>>>>>>>>>>>>>>> ValueMapperWithKey<K, V, VR>, RichFunction {
>>>>>>>>>>>>>>>>>>>>>>> ......
>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> So, we check the type of object, whether it is
>>>>>>>>>>> RichXXX
>>>>>>>>>>>>> or
>>>>>>>>>>>>>>>>>>> XXXWithKey
>>>>>>>>>>>>>>>>>>>>>> inside
>>>>>>>>>>>>>>>>>>>>>>> the called method and continue accordingly.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> If this is ok with the community, I would like
>>>>>>>>> to
>>>>>>>>>>>> revert
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>> current
>>>>>>>>>>>>>>>>>>>>>> design
>>>>>>>>>>>>>>>>>>>>>>> to this again.
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>> On Wed, Sep 13, 2017 at 3:02 PM Damian Guy <
>>>>>>>>>>>>>>>>> damian....@gmail.com
>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Hi Jeyhun,
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Thanks for sending out the update. I guess i
>>>>>>>>> was
>>>>>>>>>>>>>> thinking
>>>>>>>>>>>>>>>>> more
>>>>>>>>>>>>>>>>>>>> along
>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>> lines of option 2 where we collapse the
>>>>>>>>> RichXXXX
>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>> ValueXXXXWithKey
>>>>>>>>>>>>>>>>>>>>>> etc
>>>>>>>>>>>>>>>>>>>>>>>> interfaces into 1 interface that has all of
>>>>>>>>> the
>>>>>>>>>>>>>>> arguments. I
>>>>>>>>>>>>>>>>>>> think
>>>>>>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>>>>>>>> then
>>>>>>>>>>>>>>>>>>>>>>>> only need to add one additional overload for
>>>>>>>>>> each
>>>>>>>>>>>>>>> operator?
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>> Damian
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> On Wed, 13 Sep 2017 at 10:59 Jeyhun Karimov <
>>>>>>>>>>>>>>>>>>> je.kari...@gmail.com>
>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Dear all,
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> I would like to resume the discussion on
>>>>>>>>>>> KIP-159.
>>>>>>>>>>>> I
>>>>>>>>>>>>>> (and
>>>>>>>>>>>>>>>>>>>> Guozhang)
>>>>>>>>>>>>>>>>>>>>>>> think
>>>>>>>>>>>>>>>>>>>>>>>>> that releasing KIP-149 and KIP-159 in the
>>>>>>>>> same
>>>>>>>>>>>>> release
>>>>>>>>>>>>>>>>> would
>>>>>>>>>>>>>>>>>>> make
>>>>>>>>>>>>>>>>>>>>>> sense
>>>>>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>>>> avoid a release with "partial" public APIs.
>>>>>>>>>>> There
>>>>>>>>>>>>> is a
>>>>>>>>>>>>>>> KIP
>>>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>>>>>>>> proposed
>>>>>>>>>>>>>>>>>>>>>>>> by
>>>>>>>>>>>>>>>>>>>>>>>>> Guozhang (and approved by me) to unify both
>>>>>>>>>>> KIPs.
>>>>>>>>>>>>>>>>>>>>>>>>> Please feel free to comment on this.
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>> https://cwiki.apache.org/
>>>>>>>>>>> confluence/pages/viewpage.
>>>>>>>>>>>>>>>>>>>>>>> action?pageId=73637757
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Jul 21, 2017 at 2:00 AM Jeyhun
>>>>>>>>>> Karimov <
>>>>>>>>>>>>>>>>>>>>> je.kari...@gmail.com
>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Matthias, Damian, all,
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for your comments and sorry for
>>>>>>>>>>>> super-late
>>>>>>>>>>>>>>>>> update.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Sure, the DSL refactoring is not blocking
>>>>>>>>>> for
>>>>>>>>>>>> this
>>>>>>>>>>>>>>> KIP.
>>>>>>>>>>>>>>>>>>>>>>>>>> I made some changes to KIP document based
>>>>>>>>> on
>>>>>>>>>>> my
>>>>>>>>>>>>>>>>> prototype.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Please feel free to comment.
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Jul 7, 2017 at 9:35 PM Matthias J.
>>>>>>>>>>> Sax <
>>>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io>
>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> I would not block this KIP with regard to
>>>>>>>>>> DSL
>>>>>>>>>>>>>>>>> refactoring.
>>>>>>>>>>>>>>>>>>>> IMHO,
>>>>>>>>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>>>>>>>>> can
>>>>>>>>>>>>>>>>>>>>>>>>>>> just finish this one and the DSL
>>>>>>>>>> refactoring
>>>>>>>>>>>> will
>>>>>>>>>>>>>>> help
>>>>>>>>>>>>>>>>>> later
>>>>>>>>>>>>>>>>>>>> on
>>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>>>>>> reduce the number of overloads.
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias
>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>> On 7/7/17 5:28 AM, Jeyhun Karimov wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>> I am following the related thread in
>>>>>>>>> the
>>>>>>>>>>>>> mailing
>>>>>>>>>>>>>>> list
>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>> looking
>>>>>>>>>>>>>>>>>>>>>>>>>>> forward
>>>>>>>>>>>>>>>>>>>>>>>>>>>> for one-shot solution for overloads
>>>>>>>>>> issue.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Jul 7, 2017 at 10:32 AM Damian
>>>>>>>>>> Guy
>>>>>>>>>>> <
>>>>>>>>>>>>>>>>>>>>>> damian....@gmail.com>
>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Jeyhun,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> About overrides, what other
>>>>>>>>> alternatives
>>>>>>>>>>> do
>>>>>>>>>>>> we
>>>>>>>>>>>>>>> have?
>>>>>>>>>>>>>>>>>> For
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> backwards-compatibility we have to
>>>>>>>>> add
>>>>>>>>>>>> extra
>>>>>>>>>>>>>>>>> methods
>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>> existing
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ones.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It wasn't clear to me in the KIP if
>>>>>>>>>> these
>>>>>>>>>>>> are
>>>>>>>>>>>>>> new
>>>>>>>>>>>>>>>>>> methods
>>>>>>>>>>>>>>>>>>>> or
>>>>>>>>>>>>>>>>>>>>>>>>> replacing
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> existing ones.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Also, we are currently discussing
>>>>>>>>>> options
>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>> replacing
>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>> overrides.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Damian
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> About ProcessorContext vs
>>>>>>>>>> RecordContext,
>>>>>>>>>>>> you
>>>>>>>>>>>>>> are
>>>>>>>>>>>>>>>>>> right.
>>>>>>>>>>>>>>>>>>> I
>>>>>>>>>>>>>>>>>>>>>> think
>>>>>>>>>>>>>>>>>>>>>>> I
>>>>>>>>>>>>>>>>>>>>>>>>>>> need to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> implement a prototype to understand
>>>>>>>>> the
>>>>>>>>>>>> full
>>>>>>>>>>>>>>>>> picture
>>>>>>>>>>>>>>>>>> as
>>>>>>>>>>>>>>>>>>>> some
>>>>>>>>>>>>>>>>>>>>>>> parts
>>>>>>>>>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> KIP might not be as straightforward
>>>>>>>>> as
>>>>>>>>>> I
>>>>>>>>>>>>>> thought.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Jul 5, 2017 at 10:40 AM
>>>>>>>>> Damian
>>>>>>>>>>> Guy
>>>>>>>>>>>> <
>>>>>>>>>>>>>>>>>>>>>>> damian....@gmail.com>
>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> HI Jeyhun,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Is the intention that these methods
>>>>>>>>>> are
>>>>>>>>>>>> new
>>>>>>>>>>>>>>>>> overloads
>>>>>>>>>>>>>>>>>>> on
>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>> KStream,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> KTable, etc?
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It is worth noting that a
>>>>>>>>>>> ProcessorContext
>>>>>>>>>>>>> is
>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>>>> a
>>>>>>>>>>>>>>>>>>>>>>>> RecordContext.
>>>>>>>>>>>>>>>>>>>>>>>>> A
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RecordContext, as it stands, only
>>>>>>>>>> exists
>>>>>>>>>>>>>> during
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>> processing
>>>>>>>>>>>>>>>>>>>>>>>> of a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> single
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> record. Whereas the ProcessorContext
>>>>>>>>>>>> exists
>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>> lifetime
>>>>>>>>>>>>>>>>>>>>>> of
>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Processor. Sot it doesn't make sense
>>>>>>>>>> to
>>>>>>>>>>>>> cast a
>>>>>>>>>>>>>>>>>>>>>> ProcessorContext
>>>>>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>>>> a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RecordContext.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> You mentioned above passing the
>>>>>>>>>>>>>>>>>>> InternalProcessorContext
>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>> init()
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> calls. It is internal for a reason
>>>>>>>>>> and i
>>>>>>>>>>>>> think
>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>>>>> should
>>>>>>>>>>>>>>>>>>>>>> remain
>>>>>>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> way.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> It might be better to move the
>>>>>>>>>>>>> recordContext()
>>>>>>>>>>>>>>>>> method
>>>>>>>>>>>>>>>>>>>> from
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> InternalProcessorContext to
>>>>>>>>>>>>> ProcessorContext.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> In the KIP you have an example
>>>>>>>>>> showing:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> richMapper.init((RecordContext)
>>>>>>>>>>>>>>> processorContext);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> But the interface is:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> public interface RichValueMapper<V,
>>>>>>>>>> VR>
>>>>>>>>>>> {
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> VR apply(final V value, final
>>>>>>>>>>>>>> RecordContext
>>>>>>>>>>>>>>>>>>>>>> recordContext);
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> }
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> i.e., there is no init(...), besides
>>>>>>>>>> as
>>>>>>>>>>>>> above
>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>>>> wouldn't
>>>>>>>>>>>>>>>>>>>>>>> make
>>>>>>>>>>>>>>>>>>>>>>>>>>> sense.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Damian
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Tue, 4 Jul 2017 at 23:30 Jeyhun
>>>>>>>>>>>> Karimov <
>>>>>>>>>>>>>>>>>>>>>>> je.kari...@gmail.com
>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hi Matthias,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Actually my intend was to provide
>>>>>>>>> to
>>>>>>>>>>>>>>>>> RichInitializer
>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>> later
>>>>>>>>>>>>>>>>>>>>>>>> on
>>>>>>>>>>>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> could
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> provide the context of the record
>>>>>>>>> as
>>>>>>>>>>> you
>>>>>>>>>>>>> also
>>>>>>>>>>>>>>>>>>> mentioned.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I remove that not to confuse the
>>>>>>>>>> users.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Regarding the RecordContext and
>>>>>>>>>>>>>>> ProcessorContext
>>>>>>>>>>>>>>>>>>>>>> interfaces, I
>>>>>>>>>>>>>>>>>>>>>>>>> just
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> realized the
>>>>>>>>> InternalProcessorContext
>>>>>>>>>>>>> class.
>>>>>>>>>>>>>>>>> Can't
>>>>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>>>>> pass
>>>>>>>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>>>>>>>>> as a
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> parameter to init() method of
>>>>>>>>>>> processors?
>>>>>>>>>>>>>> Then
>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>>>> would
>>>>>>>>>>>>>>>>>>>> be
>>>>>>>>>>>>>>>>>>>>>>> able
>>>>>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> get
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> RecordContext easily with just a
>>>>>>>>>> method
>>>>>>>>>>>>> call.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Jun 29, 2017 at 10:14 PM
>>>>>>>>>>> Matthias
>>>>>>>>>>>>> J.
>>>>>>>>>>>>>>> Sax
>>>>>>>>>>>>>>>>> <
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> One more thing:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I don't think `RichInitializer`
>>>>>>>>> does
>>>>>>>>>>>> make
>>>>>>>>>>>>>>>>> sense. As
>>>>>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>>>>>>> don't
>>>>>>>>>>>>>>>>>>>>>>>> have
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> any
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> input record, there is also no
>>>>>>>>>>> context.
>>>>>>>>>>>> We
>>>>>>>>>>>>>>>>> could of
>>>>>>>>>>>>>>>>>>>>> course
>>>>>>>>>>>>>>>>>>>>>>>>> provide
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> context of the record that
>>>>>>>>> triggers
>>>>>>>>>>> the
>>>>>>>>>>>>> init
>>>>>>>>>>>>>>>>> call,
>>>>>>>>>>>>>>>>>>> but
>>>>>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>>>>>>>> seems
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> semantically questionable. Also,
>>>>>>>>> the
>>>>>>>>>>>>> context
>>>>>>>>>>>>>>> for
>>>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>>>>>>> first
>>>>>>>>>>>>>>>>>>>>>>>>> record
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> be provided by the consecutive
>>>>>>>>> call
>>>>>>>>>> to
>>>>>>>>>>>>>>> aggregate
>>>>>>>>>>>>>>>>>>>> anyways.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On 6/29/17 1:11 PM, Matthias J.
>>>>>>>>> Sax
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for updating the KIP.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I have one concern with regard to
>>>>>>>>>>>>> backward
>>>>>>>>>>>>>>>>>>>>> compatibility.
>>>>>>>>>>>>>>>>>>>>>>> You
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> suggest
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> use RecrodContext as base
>>>>>>>>> interface
>>>>>>>>>>> for
>>>>>>>>>>>>>>>>>>>>> ProcessorContext.
>>>>>>>>>>>>>>>>>>>>>>> This
>>>>>>>>>>>>>>>>>>>>>>>>>>>>> will
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> break compatibility.
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
>
signature.asc
Description: OpenPGP digital signature
