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