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 >>>> >>> >> >
signature.asc
Description: OpenPGP digital signature
