Thanks Jeyhun.

Can you also update the KIP accordingly. It must contain all changes to
public API. Thus, list all parameters that get deprecated and newly
added. And add a sentence about backward compatibility.


-Matthias

On 3/23/17 3:16 AM, Jeyhun Karimov wrote:
> Sorry for a super late update. I made an update on related PR.
> 
> Cheers,
> Jeyhun
> 
> On Wed, Mar 22, 2017 at 9:09 PM Guozhang Wang <wangg...@gmail.com> wrote:
> 
>> Jeyhun,
>>
>> Could you update the status of this KIP since it has been some time since
>> the last vote?
>>
>> I'm +1 besides the minor comments mentioned above.
>>
>>
>> Guozhang
>>
>>
>> On Mon, Mar 6, 2017 at 3:14 PM, Jeyhun Karimov <je.kari...@gmail.com>
>> wrote:
>>
>>> Sorry I was late. I will update javadocs in related methods to emphasize
>>> that TimestampExtractor is stateless.
>>>
>>> Cheers,
>>> Jeyhun
>>>
>>> On Mon, Mar 6, 2017 at 8:17 PM Guozhang Wang <wangg...@gmail.com> wrote:
>>>
>>>> 1) Sounds good.
>>>>
>>>> 2) Yeah what I meant is to emphasize that TimestampExtractor to be
>>>> stateless in the docs somewhere.
>>>>
>>>>
>>>> Guozhang
>>>>
>>>>
>>>> On Sun, Mar 5, 2017 at 4:27 PM, Matthias J. Sax <matth...@confluent.io
>>>
>>>> wrote:
>>>>
>>>>> Guozhang,
>>>>>
>>>>> about renaming the config parameters. I like this idea, but want to
>>>>> point out, that this change should be done in a backward compatible
>>> way.
>>>>> Thus, we need to keep (and only deprecate) the current parameter
>> names.
>>>>>
>>>>> I am not sure about (2)? What do you worry about? Using a "stateful
>>>>> extractor"? -- this would be an antipattern IMHO. We can clarify
>> that a
>>>>> TimestampExtrator should be stateless though (even if this should be
>>>>> clear).
>>>>>
>>>>>
>>>>> -Matthias
>>>>>
>>>>>
>>>>> On 3/4/17 6:36 PM, Guozhang Wang wrote:
>>>>>> Jeyhun,
>>>>>>
>>>>>> Thanks for proposing this KIP! And sorry for getting late in the
>>>>> discussion.
>>>>>>
>>>>>> I have a general suggestion not directly related to this KIP and a
>>>> couple
>>>>>> of comments for this KIP here:
>>>>>>
>>>>>> I agree with Mathieu's observation, partly because we are now
>> having
>>>> lots
>>>>>> of overloaded functions both in the DSL and in PAPI, and it would
>> be
>>>>> quite
>>>>>> confusing to users. As Matthias mentioned we do have some plans to
>>>>> refactor
>>>>>> this API, but just wanted to point it out that this KIP may likely
>>> urge
>>>>> us
>>>>>> to do the API refactoring sooner than planned. My personal
>> preference
>>>>> would
>>>>>> be doing that the next release (i.e. 0.11.0.0 in June).
>>>>>>
>>>>>>
>>>>>> Now some detailed comments:
>>>>>>
>>>>>> 1. I'd suggest change TIMESTAMP_EXTRACTOR_CLASS_CONFIG to
>>>>>> "default.timestamp.extractor" or "global.timestamp.extractor" (also
>>> the
>>>>>> Java variable name can be changed accordingly) along with this
>>> change.
>>>> In
>>>>>> addition, maybe we can piggy-backing this to also rename
>>>>>> KEY_SERDE_CLASS_CONFIG/VALUE_SERDE_CLASS_CONFIG to "default.key.."
>>> etc
>>>>> in
>>>>>> this KIP.
>>>>>>
>>>>>> 2. Another thing we should consider, is that since now we could
>>>>> potentially
>>>>>> use multiple timestamp extractor instances than a single one, this
>>> may
>>>> be
>>>>>> breaking if user's customization did some global bookkeeping based
>> on
>>>> the
>>>>>> previous assumption (maybe a wild thought but e.g. keeping track
>> some
>>>>>> global counts in the extractor as a local variable). We need to
>>> clarify
>>>>>> this change in the javadoc and also potentially in the upgrade web
>>> doc
>>>>>> sections.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Guozhang
>>>>>>
>>>>>>
>>>>>> On Wed, Mar 1, 2017 at 6:09 AM, Michael Noll <mich...@confluent.io
>>>
>>>>> wrote:
>>>>>>
>>>>>>> +1 (non-binding)
>>>>>>>
>>>>>>> Thanks for the KIP!
>>>>>>>
>>>>>>> On Wed, Mar 1, 2017 at 1:49 PM, Bill Bejeck <bbej...@gmail.com>
>>>> wrote:
>>>>>>>
>>>>>>>> +1
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>> Bill
>>>>>>>>
>>>>>>>> On Wed, Mar 1, 2017 at 5:06 AM, Eno Thereska <
>>> eno.there...@gmail.com
>>>>>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> +1 (non binding).
>>>>>>>>>
>>>>>>>>> Thanks
>>>>>>>>> Eno
>>>>>>>>>> On 28 Feb 2017, at 17:22, Matthias J. Sax <
>> matth...@confluent.io
>>>>
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> +1
>>>>>>>>>>
>>>>>>>>>> Thanks a lot for the KIP!
>>>>>>>>>>
>>>>>>>>>> -Matthias
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 2/28/17 1:35 AM, Damian Guy wrote:
>>>>>>>>>>> Thanks for the KIP Jeyhun!
>>>>>>>>>>>
>>>>>>>>>>> +1
>>>>>>>>>>>
>>>>>>>>>>> On Tue, 28 Feb 2017 at 08:59 Jeyhun Karimov <
>>> je.kari...@gmail.com
>>>>>
>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Dear community,
>>>>>>>>>>>>
>>>>>>>>>>>> I'd like to start the vote for KIP-123:
>>>>>>>>>>>> https://cwiki.apache.org/confluence/pages/viewpage.
>>>>>>>>> action?pageId=68714788
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Cheers,
>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>> --
>>>>>>>>>>>> -Cheers
>>>>>>>>>>>>
>>>>>>>>>>>> Jeyhun
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> -- Guozhang
>>>>
>>> --
>>> -Cheers
>>>
>>> Jeyhun
>>>
>>
>>
>>
>> --
>> -- Guozhang
>>

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to