+1 (non-binding) Thanks Eno
> On 24 Mar 2017, at 03:37, Matthias J. Sax <matth...@confluent.io> wrote: > > 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 >>> >
