+1 (non-binding) On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang <wangg...@gmail.com> wrote:
> Hello Shawn, > > Thanks for the writeup. I've made a pass over it and here are some minor > comments: > > 1) As we discussed in the PR: https://github.com/apache/kafka/pull/5307, > the public APIs that we will add is > > In WindowedSerdes: > ``` > static public <T> Serde<Windowed<T>> timeWindowedChangelogSerdeFrom(final > Class<T> type, final long windowSize) > ``` > > In TimeWindowedSerde > ``` > TimeWindowedSerde forChangelog(final boolean); > ``` > > Other classes such as WindowedKeySchema are internal classes for > implementation details and hence do not need to be listed in the wiki as > public APIs. > > > 2) The wiki doc may reads a bit confusing for audience who are not familiar > with the PR, since we mentioned the "forChangelog()" function and the > "isChangelog" parameter without clear definitions, but only explained what > it is later in the docs as java code examples. I think rephrasing the early > paragraphs to explain a bit more why we will add a new internal field along > with a setter, its semantics (its default value and how deserialization > will be different depending on that) would be better. > > Otherwise, I'm +1 on the KIP, thanks! > > > Guozhang > > > On Wed, Dec 5, 2018 at 8:18 AM Shawn Nguyen <shavvnnngu...@gmail.com> > wrote: > > > Hey all, > > > > I wanted to start a vote on approval of KIP-393 > > < > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-393%3A+Time+windowed+serde+to+properly+deserialize+changelog+input+topic > > > > > to > > fix the current time windowed serde for properly deserializing changelog > > input topics. Let me know what you guys think. > > > > Thanks, > > Shawn > > > > > -- > -- Guozhang > -- Liquan Pei Software Engineer, Confluent Inc
