Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
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 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 > 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 > > > > wrote: > > > > > >> +1 (non-binding) from me. Looks like a pretty clear-cut case. > > >> > > >> > > >> > > >> On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen > > > >> 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 > wrote: > > >>> > > +1 (non-binding) > > > > On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang > > >> 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 Serde> > > >>> timeWindowedChangelogSerdeFrom(final > > > Class 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 >
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
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 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 > > wrote: > > > >> +1 (non-binding) from me. Looks like a pretty clear-cut case. > >> > >> > >> > >> On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen > >> 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 wrote: > >>> > +1 (non-binding) > > On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang > >> 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 Serde> > >>> timeWindowedChangelogSerdeFrom(final > > Class 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 > > > 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
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
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 > wrote: > >> +1 (non-binding) from me. Looks like a pretty clear-cut case. >> >> >> >> On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen >> 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 wrote: >>> +1 (non-binding) On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang >> 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 Serde> >>> timeWindowedChangelogSerdeFrom(final > Class 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 > 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
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
+1 (binding) On Wed, 12 Dec 2018 at 13:27, Adam Bellemare wrote: > +1 (non-binding) from me. Looks like a pretty clear-cut case. > > > > On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen > 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 wrote: > > > > > +1 (non-binding) > > > > > > On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang > 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 Serde> > > timeWindowedChangelogSerdeFrom(final > > >> Class 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 > > >> 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 > > > > > >
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
+1 (non-binding) from me. Looks like a pretty clear-cut case. On Tue, Dec 11, 2018 at 1:11 AM Shawn Nguyen 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 wrote: > > > +1 (non-binding) > > > > On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang 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 Serde> > timeWindowedChangelogSerdeFrom(final > >> Class 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 > >> 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 > > >
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
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 wrote: > +1 (non-binding) > > On Wed, Dec 5, 2018 at 4:51 PM Guozhang Wang 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 Serde> timeWindowedChangelogSerdeFrom(final >> Class 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 >> 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 >
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
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 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 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%2F5307data=02%7C01%7C%7C573cbade240346489f8108d65bc0f83d%7C84df9e7fe9f640afb435%7C1%7C0%7C636797281680392086sdata=G3s1jTZI8yDak5dSJSjzJ7XnarORVuNROz1Ej932dW4%3Dreserved=0, > the public APIs that we will add is > > In WindowedSerdes: > ``` > static public Serde> timeWindowedChangelogSerdeFrom(final > Class 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 > 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%2Btopicdata=02%7C01%7C%7C573cbade240346489f8108d65bc0f83d%7C84df9e7fe9f640afb435%7C1%7C0%7C636797281680392086sdata=JOb6%2BAfEySGBQ2o5IunbZ5D3tbD3i%2FoSC7Q6mQtN388%3Dreserved=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
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 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 Serde> timeWindowedChangelogSerdeFrom(final > Class 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 > 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
Re: Vote for KIP-393 (Fix time windowed serde to deserialize changelog topic)
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 Serde> timeWindowedChangelogSerdeFrom(final Class 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 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
