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 >>
signature.asc
Description: OpenPGP digital signature
