I've made a pass on the KIP and it looks good to me as well. I think we can start a voting thread for it.
On Thu, May 30, 2019 at 10:27 PM Matthias J. Sax <matth...@confluent.io> wrote: > Thanks for the KIP Omkar. > > I think this is rather uncontroversial, and I also support this KIP. I > think you can start a VOTE. People can still chime in on the discuss > thread if they have any concerns. > > > -Matthias > > On 5/27/19 11:50 PM, Dongjin Lee wrote: > > Hi Omkar, > > > > Looks good to me. Thanks! > > > > Is there anyone who has some comments about the KIP? > > > > Thanks, > > Dongjin > > > > On Tue, May 28, 2019 at 3:24 PM omkar mestry <om.m.mes...@gmail.com> > wrote: > > > >> Hi Dongjin, > >> > >> I have updated the KIP please have a look and provide feedback on it. > >> > >> KIP :- > >> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545 > >> > >> Thanks & Regards > >> Omkar Mestry > >> > >> On Mon, May 27, 2019 at 6:25 PM Dongjin Lee <dong...@apache.org> wrote: > >> > >>> Hi Omkar, > >>> > >>> Thanks for the KIP. However, discussion thread should include a link to > >> the > >>> KIP document. Since you omitted it, here is the link. > >>> > >>> > >> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=115526545 > >>> > >>> As far as I understand, the point of the KIP is the current API can > >> result > >>> in inconsistency to should be deprecated and finally, removed. Right? > >> Here > >>> are some comments on the KIP. > >>> > >>> *1. Minor corrections* > >>> > >>> Since the KIP proposes deprecation of an API, not actually removing it, > >> it > >>> would be better to correct the following sentences: > >>> > >>> - "Therefore by removing the method put(key, value), we can prevent > >>> inconsistency." → "Therefore by deprecating (and finally removing) the > >>> method put(key, value), we can prevent inconsistency." > >>> - "Also, there are tests which are needed to be updated after removal > of > >>> the specified method." → "Also, there are tests which are needed to be > >>> updated after deprecation of the specified method." > >>> > >>> *2. About 'Motivation' section* > >>> > >>> I think the motivation section can be more clear by referring to the > risk > >>> of the current API. How do you think? > >>> > >>> "... Therefore by ..." > >>> > >>> → "... This constraint makes WindowStore error prone. Therefore by ..." > >>> > >>> *3. About 'Rejected Alternatives' section* > >>> > >>> This sections should state why these alternatives were rejected. How > >> about > >>> this? > >>> > >>> "Since this API can be called by the user[^1][^2], updating the method > >> can > >>> break the code; By this reason, this approach is not feasible." > >>> > >>> Regards, > >>> Dongjin > >>> > >>> [^1]: > >>> > >>> > >> > https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/Stores.html > >>> [^2]: > >>> > >>> > >> > https://kafka.apache.org/22/javadoc/org/apache/kafka/streams/state/WindowStore.html > >>> > >>> On Sun, May 26, 2019 at 3:18 PM omkar mestry <om.m.mes...@gmail.com> > >>> wrote: > >>> > >>>> We propose to deprecate the WindowStore#put(key, value), as it does > not > >>>> have a timestamp as a parameter. The window store requires a timestamp > >> to > >>>> map the key to a window frame. This method uses the current record > >>>> timestamp(as specified in the description of the method). There is a > >>> method > >>>> present with a timestamp as a parameter which can be used instead. > >>>> > >>> > >>> > >>> -- > >>> *Dongjin Lee* > >>> > >>> *A hitchhiker in the mathematical world.* > >>> *github: <http://goog_969573159/>github.com/dongjinleekr > >>> <https://github.com/dongjinleekr>linkedin: > >> kr.linkedin.com/in/dongjinleekr > >>> <https://kr.linkedin.com/in/dongjinleekr>speakerdeck: > >>> speakerdeck.com/dongjin > >>> <https://speakerdeck.com/dongjin>* > >>> > >> > > > > > > -- -- Guozhang
