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>* > > > -- *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>*
