Hey Shawn, thanks for the great writeup! The motivation is super clear and solution makes a lot of sense.
+1 (non-binding) Boyang ________________________________ From: Liquan Pei <liquan...@gmail.com> Sent: Friday, December 7, 2018 5:22 AM To: dev@kafka.apache.org; shavvnnngu...@gmail.com Subject: Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic) +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://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fkafka%2Fpull%2F5307&data=02%7C01%7C%7C573cbade240346489f8108d65bc0f83d%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636797281680392086&sdata=G3s1jTZI8yDak5dSJSjzJ7XnarORVuNROz1Ej932dW4%3D&reserved=0, > 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://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FKAFKA%2FKIP-393%253A%2BTime%2Bwindowed%2Bserde%2Bto%2Bproperly%2Bdeserialize%2Bchangelog%2Binput%2Btopic&data=02%7C01%7C%7C573cbade240346489f8108d65bc0f83d%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636797281680392086&sdata=JOb6%2BAfEySGBQ2o5IunbZ5D3tbD3i%2FoSC7Q6mQtN388%3D&reserved=0 > > > > > 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
