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