+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
