I'm not sure if Ismael's reply on the mailing list is a casted vote for this KIP.
@Ismael, could you review the KIP again and cast your vote if possible? Guozhang On Mon, Nov 13, 2017 at 6:10 AM, Paolo Patierno <ppatie...@live.com> wrote: > Hi all, > > I'm going to close this vote because this KIP was accepted with : > > > 3 binding votes > > 5 non-binding votes > > > Thanks everyone for comments and for voting. > > > *Paolo Patierno* > > *Senior Software Engineer (IoT) @ Red Hat **Microsoft MVP on **Azure & > IoT* > *Microsoft Azure Advisor* > > Twitter : @ppatierno <http://twitter.com/ppatierno> > Linkedin : paolopatierno <http://it.linkedin.com/in/paolopatierno> > Blog : DevExperience <http://paolopatierno.wordpress.com/> > > > ------------------------------ > *From:* Paolo Patierno <ppatie...@live.com> > *Sent:* Saturday, November 11, 2017 9:38 AM > *To:* Guozhang Wang; dev@kafka.apache.org > *Subject:* Re: [VOTE] KIP-204 : adding records deletion operation to the > new Admin Client API > > So I have updated the KIP-204<https://cwiki.apache. > org/confluence/display/KAFKA/KIP-204+%3A+Adding+records+ > deletion+operation+to+the+new+Admin+Client+API> using a DeleteRecords > class as a wrapper of the low watermark for now. > > I have updated the related PR<https://github.com/apache/kafka/pull/4132> > as well. > > > Thanks for your comments ! > > > Paolo. > > > Paolo Patierno > Senior Software Engineer (IoT) @ Red Hat > Microsoft MVP on Azure & IoT > Microsoft Azure Advisor > > Twitter : @ppatierno<http://twitter.com/ppatierno> > Linkedin : paolopatierno<http://it.linkedin.com/in/paolopatierno> > Blog : DevExperience<http://paolopatierno.wordpress.com/> > > > ________________________________ > From: Colin McCabe <co...@cmccabe.xyz> > Sent: Friday, November 10, 2017 8:08 PM > To: Guozhang Wang; dev@kafka.apache.org > Subject: Re: [VOTE] KIP-204 : adding records deletion operation to the new > Admin Client API > > +1 for returning a named object rather than Long. > > C. > > > On Fri, Nov 10, 2017, at 10:07, Guozhang Wang wrote: > > Sounds good to me for returning an object than a Long type. > > > > > > Guozhang > > > > On Fri, Nov 10, 2017 at 9:26 AM, Paolo Patierno <ppatie...@live.com> > > wrote: > > > > > Ismael, > > > > > > > > > yes it makes sense. Taking a look to the other methods in the Admin> > > Client, there is no use case returning a "simple" type : most of > > > them are> > Void or complex result represented by classes. > > > > > > In order to support future extensibility I like your idea. > > > > > > Let's see what's the others opinions otherwise I'll start to > > > implement in> > such way. > > > > > > > > > I have updated the KIP and the PR using "recordsToDelete" > > > parameter as> > well. > > > > > > > > > Thanks > > > > > > > > > Paolo Patierno > > > Senior Software Engineer (IoT) @ Red Hat > > > Microsoft MVP on Azure & IoT > > > Microsoft Azure Advisor > > > > > > Twitter : @ppatierno<http://twitter.com/ppatierno> > > > Linkedin : paolopatierno<http://it.linkedin.com/in/paolopatierno> > > > Blog : DevExperience<http://paolopatierno.wordpress.com/> > > > > > > > > > ________________________________ > > > From: isma...@gmail.com <isma...@gmail.com> on behalf of Ismael > > > Juma <> > ism...@juma.me.uk> > > > Sent: Friday, November 10, 2017 1:15 PM > > > To: dev@kafka.apache.org > > > Subject: Re: [VOTE] KIP-204 : adding records deletion operation to > > > the new> > Admin Client API > > > > > > Paolo, > > > > > > You might return the timestamp if the user did a delete by > > > timestamp for> > example. But let's maybe hear what others think > before we change > > > the KIP.> > > > > Ismael > > > > > > On Fri, Nov 10, 2017 at 12:48 PM, Paolo Patierno > > > <ppatie...@live.com>> > wrote: > > > > > > > Hi Ismael, > > > > > > > > > > > > 1. yes it sounds better. Agree. > > > > 2. We are using a wrapper class like RecordsToDelete in order > > > > to allow> > > future operations that won't be based only on > specifying an > > > > offset. At> > same > > > > time with this wrapper class and static methods (i.e. > > > > beforeOffset) the> > API > > > > is really clear to understand. About moving to use a class for > > > > wrapping> > the > > > > lowWatermark and not just a Long, do you think that in the > > > > future we> > could > > > > have some other information to bring as part of the delete > > > > operation ? or> > > just for being clearer in terms of API (so not > just with a > > > > comment) ?> > > > > > > Thanks, > > > > Paolo. > > > > > > > > > > > > Paolo Patierno > > > > Senior Software Engineer (IoT) @ Red Hat > > > > Microsoft MVP on Azure & IoT > > > > Microsoft Azure Advisor > > > > > > > > Twitter : @ppatierno<http://twitter.com/ppatierno> > > > > Linkedin : paolopatierno<http://it.linkedin.com/in/paolopatierno>> > > > Blog : DevExperience<http://paolopatierno.wordpress.com/> > > > > > > > > > > > > ________________________________ > > > > From: isma...@gmail.com <isma...@gmail.com> on behalf of Ismael > > > > Juma <> > > ism...@juma.me.uk> > > > > Sent: Friday, November 10, 2017 12:33 PM > > > > To: dev@kafka.apache.org > > > > Subject: Re: [VOTE] KIP-204 : adding records deletion operation > > > > to the> > new > > > > Admin Client API > > > > > > > > Hi Paolo, > > > > > > > > The KIP looks good, I have a couple of comments: > > > > > > > > 1. partitionsAndOffsets could perhaps be `recordsToDelete`. > > > > 2. It seems a bit inconsistent that the argument is > > > > `RecordsToDelete`,> > but > > > > the result is just a `Long`. Should the result be > > > > `DeleteRecords` or> > > something like that? It could then have a > field logStartOffset or> > > lowWatermark instead of having to document it > via a comment only.> > > > > > > Ismael > > > > > > > > On Tue, Oct 3, 2017 at 6:51 AM, Paolo Patierno > > > > <ppatie...@live.com>> > wrote: > > > > > > > > > Hi all, > > > > > > > > > > I didn't see any further discussion around this KIP, so I'd like > > > > > to> > start > > > > > the vote for it. > > > > > > > > > > Just for reference : https://cwiki.apache.org/confl > > > > > uence/display/KAFKA/KIP-204+%3A+adding+records+deletion+ > > > > > operation+to+the+new+Admin+Client+API > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > Paolo Patierno > > > > > Senior Software Engineer (IoT) @ Red Hat > > > > > Microsoft MVP on Azure & IoT > > > > > Microsoft Azure Advisor > > > > > > > > > > Twitter : @ppatierno<http://twitter.com/ppatierno> > > > > > Linkedin : paolopatierno > > > > > <http://it.linkedin.com/in/paolopatierno>> > > > Blog : > DevExperience<http://paolopatierno.wordpress.com/> > > > > > > > > > > > > > > > > > > > > -- > > -- Guozhang > > -- -- Guozhang
