kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Guozhang Wang <wangg...@gmail.com>
Subject Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
Date Fri, 04 Jan 2019 19:35:24 GMT
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 <matthias@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.bellemare@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 <shavvnnnguyen@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 <liquanpei@gmail.com> wrote:
> >>>
> >>>> +1 (non-binding)
> >>>>
> >>>> On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang <wangguoz@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 <shavvnnnguyen@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message