Thanks Guozhang and Matthias! https://github.com/apache/kafka/pull/5307 is merged now.
We can close this thread now since we have a total of 3 binding votes (and 3 additional non-binding votes). On Fri, Jan 4, 2019 at 11:59 AM Guozhang Wang <wangg...@gmail.com> wrote: > Hello Shawn, > > Could you close this voting thread with a tally on the votes (you can > search for other voting threads as a reference)? > > > Guozhang > > On Wed, Dec 19, 2018 at 1:15 AM Matthias J. Sax <matth...@confluent.io> > wrote: > > > Sorry for the delay. Needed to read the KIP and code multiple times to > > wrap my head around it. > > > > Thanks for the KIP. > > > > > > +1 (binding) > > > > > > -Matthias > > > > > > On 12/13/18 1:19 PM, Damian Guy wrote: > > > +1 (binding) > > > > > > On Wed, 12 Dec 2018 at 13:27, Adam Bellemare <adam.bellem...@gmail.com > > > > > wrote: > > > > > >> +1 (non-binding) from me. Looks like a pretty clear-cut case. > > >> > > >> > > >> > > >> On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen <shavvnnngu...@gmail.com > > > > >> wrote: > > >> > > >>> Thanks for the feedback Guozhang! I updated the KIP. > > >>> > > >>> In the meantime, could I ask for additional binding votes/approval on > > >> this > > >>> KIP proposal? > > >>> > > >>> Shawn > > >>> > > >>> On Thu, Dec 6, 2018 at 1:22 PM Liquan Pei <liquan...@gmail.com> > wrote: > > >>> > > >>>> +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 > > >>>> > > >>> > > >> > > > > > > > > > -- > -- Guozhang >
