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/> > > >
