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

Reply via email to