Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Colin,


I have just updated the KIP mentioning that this new method should replace the 
"legacy" Scala API used for deleting records today.


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: Paolo Patierno <ppatie...@live.com>
Sent: Tuesday, October 31, 2017 8:56 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Colin,
thanks !

This morning (Italy time zone) I started the vote for this KIP. Up to now there 
are 5 non-binding votes.

In any case, I'll update the section you mentioned. I totally agree with you on 
giving more info to developers who are using the Scala API.

Thanks
Paolo

From: Colin McCabe <cmcc...@apache.org>
Sent: Tuesday, October 31, 2017 9:49:59 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

This looks like a good proposal.  I think it's probably ready to take it
to a vote soon?

Also, in the "Compatibility, Deprecation, and Migration Plan" section,
you might want to mention the internal Scala interface for doing this
which was added in KIP-107.  We should expect users to migrate from that
interface to the new one over time.

best,
Colin


On Wed, Oct 25, 2017, at 03:47, Paolo Patierno wrote:
> Thanks for all your feedback guys. I have updated my current code as
> well.
>
> I know that the vote for this KIP is not started yet (actually I opened
> it due to no feedback on this KIP after a while but then the discussion
> started and it was really useful !) but I have already opened a PR for
> that.
>
> Maybe feedback could be useful on that as well :
>
>
> https://github.com/apache/kafka/pull/4132
>
>
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Monday, October 23, 2017 4:34 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> On Mon, Oct 23, 2017, at 01:37, Tom Bentley wrote:
> > At the risk of muddying the waters further, have you considered
> > "RecordsToDelete" as the name of the class? It's both shorter and more
> > descriptive imho.
>
> +1 for RecordsToDelete
>
> >
> > Also "deleteBefore()" as the factory method name isn't very future proof
> > if
> > we came to support time-based deletion. Something like "beforeOffset()"
> > would be clearer, imho.
>
> Great idea.
>
> best,
> Colin
>
> >
> > Putting these together: RecordsToDelete.beforeOffset() seems much clearer
> > to me than DeleteRecordsTarget.deleteBefore()
> >
> >
> > On 23 October 2017 at 08:45, Paolo Patierno <ppatie...@live.com> wrote:
> >
> > > About the name I just started to have a doubt about DeletetionTarget
> > > because it could be bounded to any deletion operation (i.e. delete topic,
> > > ...) and not just what we want now, so records deletion.
> > >
> > > I have updated the KIP-204 using DeleteRecordsTarget so it's clear that
> > > it's related to the delete records operation and what it means, so the
> > > target for such operation.
> > >
> > >
> > > 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: Monday, October 23, 2017 7:38 AM
> > > To: dev@kafka.apache.org
> > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > > new Admin Client API
> > >
> > > Hi Colin,
> > >
> >

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Colin,
thanks !

This morning (Italy time zone) I started the vote for this KIP. Up to now there 
are 5 non-binding votes.

In any case, I'll update the section you mentioned. I totally agree with you on 
giving more info to developers who are using the Scala API.

Thanks
Paolo

From: Colin McCabe <cmcc...@apache.org>
Sent: Tuesday, October 31, 2017 9:49:59 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

This looks like a good proposal.  I think it's probably ready to take it
to a vote soon?

Also, in the "Compatibility, Deprecation, and Migration Plan" section,
you might want to mention the internal Scala interface for doing this
which was added in KIP-107.  We should expect users to migrate from that
interface to the new one over time.

best,
Colin


On Wed, Oct 25, 2017, at 03:47, Paolo Patierno wrote:
> Thanks for all your feedback guys. I have updated my current code as
> well.
>
> I know that the vote for this KIP is not started yet (actually I opened
> it due to no feedback on this KIP after a while but then the discussion
> started and it was really useful !) but I have already opened a PR for
> that.
>
> Maybe feedback could be useful on that as well :
>
>
> https://github.com/apache/kafka/pull/4132
>
>
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Monday, October 23, 2017 4:34 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> On Mon, Oct 23, 2017, at 01:37, Tom Bentley wrote:
> > At the risk of muddying the waters further, have you considered
> > "RecordsToDelete" as the name of the class? It's both shorter and more
> > descriptive imho.
>
> +1 for RecordsToDelete
>
> >
> > Also "deleteBefore()" as the factory method name isn't very future proof
> > if
> > we came to support time-based deletion. Something like "beforeOffset()"
> > would be clearer, imho.
>
> Great idea.
>
> best,
> Colin
>
> >
> > Putting these together: RecordsToDelete.beforeOffset() seems much clearer
> > to me than DeleteRecordsTarget.deleteBefore()
> >
> >
> > On 23 October 2017 at 08:45, Paolo Patierno <ppatie...@live.com> wrote:
> >
> > > About the name I just started to have a doubt about DeletetionTarget
> > > because it could be bounded to any deletion operation (i.e. delete topic,
> > > ...) and not just what we want now, so records deletion.
> > >
> > > I have updated the KIP-204 using DeleteRecordsTarget so it's clear that
> > > it's related to the delete records operation and what it means, so the
> > > target for such operation.
> > >
> > >
> > > 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: Monday, October 23, 2017 7:38 AM
> > > To: dev@kafka.apache.org
> > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > > new Admin Client API
> > >
> > > Hi Colin,
> > >
> > > I was using the long primitive in the code but not updated the KIP yet,
> > > sorry ... now it's updated !
> > >
> > > At same time I agree on using DeletionTarget ... KIP updated !
> > >
> > >
> > > Regarding the deleteBefore factory method, it's a pattern already used
> > > witn NewPartitions.increaseTo which I think it's really clear and give us
> > > more possibility to evolve this DeletionTarget class if we'll add 
> > > different
> > > ways to specify such target not only offset based.
> > >
> > >
> > > Thanks,
> > >
> > >
> > > Paolo Patierno
> > > Sen

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Colin McCabe]

Hi Paolo,

This looks like a good proposal.  I think it's probably ready to take it
to a vote soon?

Also, in the "Compatibility, Deprecation, and Migration Plan" section,
you might want to mention the internal Scala interface for doing this
which was added in KIP-107.  We should expect users to migrate from that
interface to the new one over time.

best,
Colin


On Wed, Oct 25, 2017, at 03:47, Paolo Patierno wrote:
> Thanks for all your feedback guys. I have updated my current code as
> well.
> 
> I know that the vote for this KIP is not started yet (actually I opened
> it due to no feedback on this KIP after a while but then the discussion
> started and it was really useful !) but I have already opened a PR for
> that.
> 
> Maybe feedback could be useful on that as well :
> 
> 
> https://github.com/apache/kafka/pull/4132
> 
> 
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Monday, October 23, 2017 4:34 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
> 
> On Mon, Oct 23, 2017, at 01:37, Tom Bentley wrote:
> > At the risk of muddying the waters further, have you considered
> > "RecordsToDelete" as the name of the class? It's both shorter and more
> > descriptive imho.
> 
> +1 for RecordsToDelete
> 
> >
> > Also "deleteBefore()" as the factory method name isn't very future proof
> > if
> > we came to support time-based deletion. Something like "beforeOffset()"
> > would be clearer, imho.
> 
> Great idea.
> 
> best,
> Colin
> 
> >
> > Putting these together: RecordsToDelete.beforeOffset() seems much clearer
> > to me than DeleteRecordsTarget.deleteBefore()
> >
> >
> > On 23 October 2017 at 08:45, Paolo Patierno <ppatie...@live.com> wrote:
> >
> > > About the name I just started to have a doubt about DeletetionTarget
> > > because it could be bounded to any deletion operation (i.e. delete topic,
> > > ...) and not just what we want now, so records deletion.
> > >
> > > I have updated the KIP-204 using DeleteRecordsTarget so it's clear that
> > > it's related to the delete records operation and what it means, so the
> > > target for such operation.
> > >
> > >
> > > 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: Monday, October 23, 2017 7:38 AM
> > > To: dev@kafka.apache.org
> > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > > new Admin Client API
> > >
> > > Hi Colin,
> > >
> > > I was using the long primitive in the code but not updated the KIP yet,
> > > sorry ... now it's updated !
> > >
> > > At same time I agree on using DeletionTarget ... KIP updated !
> > >
> > >
> > > Regarding the deleteBefore factory method, it's a pattern already used
> > > witn NewPartitions.increaseTo which I think it's really clear and give us
> > > more possibility to evolve this DeletionTarget class if we'll add 
> > > different
> > > ways to specify such target not only offset based.
> > >
> > >
> > > 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: Colin McCabe <cmcc...@apache.or

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Thanks for all your feedback guys. I have updated my current code as well.

I know that the vote for this KIP is not started yet (actually I opened it due 
to no feedback on this KIP after a while but then the discussion started and it 
was really useful !) but I have already opened a PR for that.

Maybe feedback could be useful on that as well :


https://github.com/apache/kafka/pull/4132


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: Colin McCabe <cmcc...@apache.org>
Sent: Monday, October 23, 2017 4:34 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

On Mon, Oct 23, 2017, at 01:37, Tom Bentley wrote:
> At the risk of muddying the waters further, have you considered
> "RecordsToDelete" as the name of the class? It's both shorter and more
> descriptive imho.

+1 for RecordsToDelete

>
> Also "deleteBefore()" as the factory method name isn't very future proof
> if
> we came to support time-based deletion. Something like "beforeOffset()"
> would be clearer, imho.

Great idea.

best,
Colin

>
> Putting these together: RecordsToDelete.beforeOffset() seems much clearer
> to me than DeleteRecordsTarget.deleteBefore()
>
>
> On 23 October 2017 at 08:45, Paolo Patierno <ppatie...@live.com> wrote:
>
> > About the name I just started to have a doubt about DeletetionTarget
> > because it could be bounded to any deletion operation (i.e. delete topic,
> > ...) and not just what we want now, so records deletion.
> >
> > I have updated the KIP-204 using DeleteRecordsTarget so it's clear that
> > it's related to the delete records operation and what it means, so the
> > target for such operation.
> >
> >
> > 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: Monday, October 23, 2017 7:38 AM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > Hi Colin,
> >
> > I was using the long primitive in the code but not updated the KIP yet,
> > sorry ... now it's updated !
> >
> > At same time I agree on using DeletionTarget ... KIP updated !
> >
> >
> > Regarding the deleteBefore factory method, it's a pattern already used
> > witn NewPartitions.increaseTo which I think it's really clear and give us
> > more possibility to evolve this DeletionTarget class if we'll add different
> > ways to specify such target not only offset based.
> >
> >
> > 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: Colin McCabe <cmcc...@apache.org>
> > Sent: Friday, October 20, 2017 8:18 PM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > > /** Describe records to delete */
> >  > public class DeleteRecords {
> >  > private Long offset;
> >
> > "DeleteRecords" doesn't really explain what the class is, though.  How
> > about "DeletionTarget"?  Also, why do we need a Long object rather than
> > a long primitive?
> >
> >  >
> >  > /**
> >  > * Delete all the records before the given {@code offset}
> >  > */
> >  > public static DeleteRecords deleteBefore(Long offset) { ... }
> >
> > This seems confusing to me.  What's wrong with a regular constructor for
> > DeletionTarget?
> >
> > best,
> > Colin
> >
> >
> > On Fri, Oct 20, 201

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Colin McCabe]

On Mon, Oct 23, 2017, at 01:37, Tom Bentley wrote:
> At the risk of muddying the waters further, have you considered
> "RecordsToDelete" as the name of the class? It's both shorter and more
> descriptive imho.

+1 for RecordsToDelete

> 
> Also "deleteBefore()" as the factory method name isn't very future proof
> if
> we came to support time-based deletion. Something like "beforeOffset()"
> would be clearer, imho.

Great idea.

best,
Colin

> 
> Putting these together: RecordsToDelete.beforeOffset() seems much clearer
> to me than DeleteRecordsTarget.deleteBefore()
> 
> 
> On 23 October 2017 at 08:45, Paolo Patierno <ppatie...@live.com> wrote:
> 
> > About the name I just started to have a doubt about DeletetionTarget
> > because it could be bounded to any deletion operation (i.e. delete topic,
> > ...) and not just what we want now, so records deletion.
> >
> > I have updated the KIP-204 using DeleteRecordsTarget so it's clear that
> > it's related to the delete records operation and what it means, so the
> > target for such operation.
> >
> >
> > 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: Monday, October 23, 2017 7:38 AM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > Hi Colin,
> >
> > I was using the long primitive in the code but not updated the KIP yet,
> > sorry ... now it's updated !
> >
> > At same time I agree on using DeletionTarget ... KIP updated !
> >
> >
> > Regarding the deleteBefore factory method, it's a pattern already used
> > witn NewPartitions.increaseTo which I think it's really clear and give us
> > more possibility to evolve this DeletionTarget class if we'll add different
> > ways to specify such target not only offset based.
> >
> >
> > 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: Colin McCabe <cmcc...@apache.org>
> > Sent: Friday, October 20, 2017 8:18 PM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > > /** Describe records to delete */
> >  > public class DeleteRecords {
> >  > private Long offset;
> >
> > "DeleteRecords" doesn't really explain what the class is, though.  How
> > about "DeletionTarget"?  Also, why do we need a Long object rather than
> > a long primitive?
> >
> >  >
> >  > /**
> >  > * Delete all the records before the given {@code offset}
> >  > */
> >  > public static DeleteRecords deleteBefore(Long offset) { ... }
> >
> > This seems confusing to me.  What's wrong with a regular constructor for
> > DeletionTarget?
> >
> > best,
> > Colin
> >
> >
> > On Fri, Oct 20, 2017, at 01:28, Paolo Patierno wrote:
> > > Hi all,
> > >
> > >
> > > I have just updated the KIP with your suggestion.
> > >
> > > I'm going to continue implementation and tests with these changes,
> > > waiting for further discussion.
> > >
> > >
> > > 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: Paolo Patierno <ppatie...@live.com&

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Tom Bentley]

At the risk of muddying the waters further, have you considered
"RecordsToDelete" as the name of the class? It's both shorter and more
descriptive imho.

Also "deleteBefore()" as the factory method name isn't very future proof if
we came to support time-based deletion. Something like "beforeOffset()"
would be clearer, imho.

Putting these together: RecordsToDelete.beforeOffset() seems much clearer
to me than DeleteRecordsTarget.deleteBefore()


On 23 October 2017 at 08:45, Paolo Patierno <ppatie...@live.com> wrote:

> About the name I just started to have a doubt about DeletetionTarget
> because it could be bounded to any deletion operation (i.e. delete topic,
> ...) and not just what we want now, so records deletion.
>
> I have updated the KIP-204 using DeleteRecordsTarget so it's clear that
> it's related to the delete records operation and what it means, so the
> target for such operation.
>
>
> 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: Monday, October 23, 2017 7:38 AM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Colin,
>
> I was using the long primitive in the code but not updated the KIP yet,
> sorry ... now it's updated !
>
> At same time I agree on using DeletionTarget ... KIP updated !
>
>
> Regarding the deleteBefore factory method, it's a pattern already used
> witn NewPartitions.increaseTo which I think it's really clear and give us
> more possibility to evolve this DeletionTarget class if we'll add different
> ways to specify such target not only offset based.
>
>
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Friday, October 20, 2017 8:18 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> > /** Describe records to delete */
>  > public class DeleteRecords {
>  > private Long offset;
>
> "DeleteRecords" doesn't really explain what the class is, though.  How
> about "DeletionTarget"?  Also, why do we need a Long object rather than
> a long primitive?
>
>  >
>  > /**
>  > * Delete all the records before the given {@code offset}
>  > */
>  > public static DeleteRecords deleteBefore(Long offset) { ... }
>
> This seems confusing to me.  What's wrong with a regular constructor for
> DeletionTarget?
>
> best,
> Colin
>
>
> On Fri, Oct 20, 2017, at 01:28, Paolo Patierno wrote:
> > Hi all,
> >
> >
> > I have just updated the KIP with your suggestion.
> >
> > I'm going to continue implementation and tests with these changes,
> > waiting for further discussion.
> >
> >
> > 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: Paolo Patierno <ppatie...@live.com>
> > Sent: Thursday, October 19, 2017 1:37 PM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > @Colin Yes you are right I'll update the KIP-204 mentioning the related
> > ACL permission DELETE on TOPIC
> >
> >
> > @Dong @Ismael
> >
> >
> > Considering future improvements on this, it makes sense to me using a
> > class instead of a Long.
> >
> > Maybe the name could be just "DeleteRecords" (as "NewPartitions") having
> > a deleteBefore(Long) factory method for a simple creation when you need

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

About the name I just started to have a doubt about DeletetionTarget because it 
could be bounded to any deletion operation (i.e. delete topic, ...) and not 
just what we want now, so records deletion.

I have updated the KIP-204 using DeleteRecordsTarget so it's clear that it's 
related to the delete records operation and what it means, so the target for 
such operation.


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: Monday, October 23, 2017 7:38 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Colin,

I was using the long primitive in the code but not updated the KIP yet, sorry 
... now it's updated !

At same time I agree on using DeletionTarget ... KIP updated !


Regarding the deleteBefore factory method, it's a pattern already used witn 
NewPartitions.increaseTo which I think it's really clear and give us more 
possibility to evolve this DeletionTarget class if we'll add different ways to 
specify such target not only offset based.


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: Colin McCabe <cmcc...@apache.org>
Sent: Friday, October 20, 2017 8:18 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

> /** Describe records to delete */
 > public class DeleteRecords {
 > private Long offset;

"DeleteRecords" doesn't really explain what the class is, though.  How
about "DeletionTarget"?  Also, why do we need a Long object rather than
a long primitive?

 >
 > /**
 > * Delete all the records before the given {@code offset}
 > */
 > public static DeleteRecords deleteBefore(Long offset) { ... }

This seems confusing to me.  What's wrong with a regular constructor for
DeletionTarget?

best,
Colin


On Fri, Oct 20, 2017, at 01:28, Paolo Patierno wrote:
> Hi all,
>
>
> I have just updated the KIP with your suggestion.
>
> I'm going to continue implementation and tests with these changes,
> waiting for further discussion.
>
>
> 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: Paolo Patierno <ppatie...@live.com>
> Sent: Thursday, October 19, 2017 1:37 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> @Colin Yes you are right I'll update the KIP-204 mentioning the related
> ACL permission DELETE on TOPIC
>
>
> @Dong @Ismael
>
>
> Considering future improvements on this, it makes sense to me using a
> class instead of a Long.
>
> Maybe the name could be just "DeleteRecords" (as "NewPartitions") having
> a deleteBefore(Long) factory method for a simple creation when you need
> to delete before the specified offset.
>
>
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Wednesday, October 18, 2017 3:58 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Having a class there makes sense to me.  It also helps clarify what the
> Long represents (a record offset).
>
> regards,
> Colin
>
>
> On Wed, Oct 18, 2017, at 06:19, Dong Lin wrote:
> > Sure. This makes sense. I agree it is better to replace Long with a new
> > class.
> >
> > On Wed, Oct 18, 2017 at 6:16 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> >
> &

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Colin,

I was using the long primitive in the code but not updated the KIP yet, sorry 
... now it's updated !

At same time I agree on using DeletionTarget ... KIP updated !


Regarding the deleteBefore factory method, it's a pattern already used witn 
NewPartitions.increaseTo which I think it's really clear and give us more 
possibility to evolve this DeletionTarget class if we'll add different ways to 
specify such target not only offset based.


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: Colin McCabe <cmcc...@apache.org>
Sent: Friday, October 20, 2017 8:18 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

> /** Describe records to delete */
 > public class DeleteRecords {
 > private Long offset;

"DeleteRecords" doesn't really explain what the class is, though.  How
about "DeletionTarget"?  Also, why do we need a Long object rather than
a long primitive?

 >
 > /**
 > * Delete all the records before the given {@code offset}
 > */
 > public static DeleteRecords deleteBefore(Long offset) { ... }

This seems confusing to me.  What's wrong with a regular constructor for
DeletionTarget?

best,
Colin


On Fri, Oct 20, 2017, at 01:28, Paolo Patierno wrote:
> Hi all,
>
>
> I have just updated the KIP with your suggestion.
>
> I'm going to continue implementation and tests with these changes,
> waiting for further discussion.
>
>
> 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: Paolo Patierno <ppatie...@live.com>
> Sent: Thursday, October 19, 2017 1:37 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> @Colin Yes you are right I'll update the KIP-204 mentioning the related
> ACL permission DELETE on TOPIC
>
>
> @Dong @Ismael
>
>
> Considering future improvements on this, it makes sense to me using a
> class instead of a Long.
>
> Maybe the name could be just "DeleteRecords" (as "NewPartitions") having
> a deleteBefore(Long) factory method for a simple creation when you need
> to delete before the specified offset.
>
>
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Wednesday, October 18, 2017 3:58 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Having a class there makes sense to me.  It also helps clarify what the
> Long represents (a record offset).
>
> regards,
> Colin
>
>
> On Wed, Oct 18, 2017, at 06:19, Dong Lin wrote:
> > Sure. This makes sense. I agree it is better to replace Long with a new
> > class.
> >
> > On Wed, Oct 18, 2017 at 6:16 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> >
> > > Hi Dong,
> > >
> > > Yes, I mean replacing the `Long` with a class in the map. The class would
> > > have static factory methods for the various use cases. To use the
> > > `createPartitions` example, there is a `NewPartitions.increaseTo` method.
> > >
> > > Not sure why you think it's too complicated. It provides better type
> > > safety, it's more informative and makes it easier to evolve. Thankfully
> > > Java has lambdas for a while now and mapping a collection from one type to
> > > another is reasonably simple.
> > >
> > > Your suggestion doesn't work because both methods would have the same
> > > "erased" signature. You can't have two overloaded methods that have the
> > > same signature apart from generic parameters. Also, we'd end up with 2
> > > methods in AdminClient.

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Colin McCabe]

 > /** Describe records to delete */
 > public class DeleteRecords {
 > private Long offset;

"DeleteRecords" doesn't really explain what the class is, though.  How
about "DeletionTarget"?  Also, why do we need a Long object rather than
a long primitive?

 >  
 > /**
 > * Delete all the records before the given {@code offset}
 > */
 > public static DeleteRecords deleteBefore(Long offset) { ... }

This seems confusing to me.  What's wrong with a regular constructor for
DeletionTarget?

best,
Colin


On Fri, Oct 20, 2017, at 01:28, Paolo Patierno wrote:
> Hi all,
> 
> 
> I have just updated the KIP with your suggestion.
> 
> I'm going to continue implementation and tests with these changes,
> waiting for further discussion.
> 
> 
> 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: Paolo Patierno <ppatie...@live.com>
> Sent: Thursday, October 19, 2017 1:37 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
> 
> @Colin Yes you are right I'll update the KIP-204 mentioning the related
> ACL permission DELETE on TOPIC
> 
> 
> @Dong @Ismael
> 
> 
> Considering future improvements on this, it makes sense to me using a
> class instead of a Long.
> 
> Maybe the name could be just "DeleteRecords" (as "NewPartitions") having
> a deleteBefore(Long) factory method for a simple creation when you need
> to delete before the specified offset.
> 
> 
> 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: Colin McCabe <cmcc...@apache.org>
> Sent: Wednesday, October 18, 2017 3:58 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
> 
> Having a class there makes sense to me.  It also helps clarify what the
> Long represents (a record offset).
> 
> regards,
> Colin
> 
> 
> On Wed, Oct 18, 2017, at 06:19, Dong Lin wrote:
> > Sure. This makes sense. I agree it is better to replace Long with a new
> > class.
> >
> > On Wed, Oct 18, 2017 at 6:16 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> >
> > > Hi Dong,
> > >
> > > Yes, I mean replacing the `Long` with a class in the map. The class would
> > > have static factory methods for the various use cases. To use the
> > > `createPartitions` example, there is a `NewPartitions.increaseTo` method.
> > >
> > > Not sure why you think it's too complicated. It provides better type
> > > safety, it's more informative and makes it easier to evolve. Thankfully
> > > Java has lambdas for a while now and mapping a collection from one type to
> > > another is reasonably simple.
> > >
> > > Your suggestion doesn't work because both methods would have the same
> > > "erased" signature. You can't have two overloaded methods that have the
> > > same signature apart from generic parameters. Also, we'd end up with 2
> > > methods in AdminClient.
> > >
> > > Ismael
> > >
> > >
> > > On Wed, Oct 18, 2017 at 1:42 PM, Dong Lin <lindon...@gmail.com> wrote:
> > >
> > > > Hey Ismael,
> > > >
> > > > To clarify, I think you are saying that we should replace
> > > > "deleteRecords(Map<TopicPartition, Long> partitionsAndOffsets)" with
> > > > "deleteRecords(Map<TopicPartition, DeleteRecordsParameter>
> > > > partitionsAndOffsets)", where DeleteRecordsParameter should be include a
> > > > "Long value", and probably "Boolean isBeforeOrAfter" and "Boolean
> > > > isOffsetOrTime" in the future.
> > > >
> > > > I get the point that we want to only include additional parameter
> > > > in DeleteRecordsOptions. I just feel it is a bit overkill to have a new
> > &g

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi all,


I have just updated the KIP with your suggestion.

I'm going to continue implementation and tests with these changes, waiting for 
further discussion.


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: Paolo Patierno <ppatie...@live.com>
Sent: Thursday, October 19, 2017 1:37 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

@Colin Yes you are right I'll update the KIP-204 mentioning the related ACL 
permission DELETE on TOPIC


@Dong @Ismael


Considering future improvements on this, it makes sense to me using a class 
instead of a Long.

Maybe the name could be just "DeleteRecords" (as "NewPartitions") having a 
deleteBefore(Long) factory method for a simple creation when you need to delete 
before the specified offset.


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: Colin McCabe <cmcc...@apache.org>
Sent: Wednesday, October 18, 2017 3:58 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Having a class there makes sense to me.  It also helps clarify what the
Long represents (a record offset).

regards,
Colin


On Wed, Oct 18, 2017, at 06:19, Dong Lin wrote:
> Sure. This makes sense. I agree it is better to replace Long with a new
> class.
>
> On Wed, Oct 18, 2017 at 6:16 AM, Ismael Juma <ism...@juma.me.uk> wrote:
>
> > Hi Dong,
> >
> > Yes, I mean replacing the `Long` with a class in the map. The class would
> > have static factory methods for the various use cases. To use the
> > `createPartitions` example, there is a `NewPartitions.increaseTo` method.
> >
> > Not sure why you think it's too complicated. It provides better type
> > safety, it's more informative and makes it easier to evolve. Thankfully
> > Java has lambdas for a while now and mapping a collection from one type to
> > another is reasonably simple.
> >
> > Your suggestion doesn't work because both methods would have the same
> > "erased" signature. You can't have two overloaded methods that have the
> > same signature apart from generic parameters. Also, we'd end up with 2
> > methods in AdminClient.
> >
> > Ismael
> >
> >
> > On Wed, Oct 18, 2017 at 1:42 PM, Dong Lin <lindon...@gmail.com> wrote:
> >
> > > Hey Ismael,
> > >
> > > To clarify, I think you are saying that we should replace
> > > "deleteRecords(Map<TopicPartition, Long> partitionsAndOffsets)" with
> > > "deleteRecords(Map<TopicPartition, DeleteRecordsParameter>
> > > partitionsAndOffsets)", where DeleteRecordsParameter should be include a
> > > "Long value", and probably "Boolean isBeforeOrAfter" and "Boolean
> > > isOffsetOrTime" in the future.
> > >
> > > I get the point that we want to only include additional parameter
> > > in DeleteRecordsOptions. I just feel it is a bit overkill to have a new
> > > class DeleteRecordsParameter which will only support offset in the near
> > > future. This method signature seems a bit too complicated.
> > >
> > > How about we use deleteRecords(Map<TopicPartition, Long>
> > > partitionsAndOffsets) for now, and add deleteRecords(Map<TopicPartition,
> > > DeleteRecordsParameter> partitionsAndOffsets) when we need to support
> > core
> > > parameter in the future? By doing this we can make user's experience
> > better
> > > (i.e. not having to instantiate DeleteRecordsParameter) until it is
> > > necessary to be more complicated.
> > >
> > > Dong
> > >
> > > On Wed, Oct 18, 2017 at 4:46 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> > >
> > > > Hi Dong,
> > > >
> > > > I think it makes sense to use the parameters to define the specifics of
> > > the
> > > > request. However, we would probably want to replace the `Long` with a
> > > class
> > > > (similar to `createPartitions`) instead of relying on
&

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

@Colin Yes you are right I'll update the KIP-204 mentioning the related ACL 
permission DELETE on TOPIC


@Dong @Ismael


Considering future improvements on this, it makes sense to me using a class 
instead of a Long.

Maybe the name could be just "DeleteRecords" (as "NewPartitions") having a 
deleteBefore(Long) factory method for a simple creation when you need to delete 
before the specified offset.


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: Colin McCabe <cmcc...@apache.org>
Sent: Wednesday, October 18, 2017 3:58 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Having a class there makes sense to me.  It also helps clarify what the
Long represents (a record offset).

regards,
Colin


On Wed, Oct 18, 2017, at 06:19, Dong Lin wrote:
> Sure. This makes sense. I agree it is better to replace Long with a new
> class.
>
> On Wed, Oct 18, 2017 at 6:16 AM, Ismael Juma <ism...@juma.me.uk> wrote:
>
> > Hi Dong,
> >
> > Yes, I mean replacing the `Long` with a class in the map. The class would
> > have static factory methods for the various use cases. To use the
> > `createPartitions` example, there is a `NewPartitions.increaseTo` method.
> >
> > Not sure why you think it's too complicated. It provides better type
> > safety, it's more informative and makes it easier to evolve. Thankfully
> > Java has lambdas for a while now and mapping a collection from one type to
> > another is reasonably simple.
> >
> > Your suggestion doesn't work because both methods would have the same
> > "erased" signature. You can't have two overloaded methods that have the
> > same signature apart from generic parameters. Also, we'd end up with 2
> > methods in AdminClient.
> >
> > Ismael
> >
> >
> > On Wed, Oct 18, 2017 at 1:42 PM, Dong Lin <lindon...@gmail.com> wrote:
> >
> > > Hey Ismael,
> > >
> > > To clarify, I think you are saying that we should replace
> > > "deleteRecords(Map<TopicPartition, Long> partitionsAndOffsets)" with
> > > "deleteRecords(Map<TopicPartition, DeleteRecordsParameter>
> > > partitionsAndOffsets)", where DeleteRecordsParameter should be include a
> > > "Long value", and probably "Boolean isBeforeOrAfter" and "Boolean
> > > isOffsetOrTime" in the future.
> > >
> > > I get the point that we want to only include additional parameter
> > > in DeleteRecordsOptions. I just feel it is a bit overkill to have a new
> > > class DeleteRecordsParameter which will only support offset in the near
> > > future. This method signature seems a bit too complicated.
> > >
> > > How about we use deleteRecords(Map<TopicPartition, Long>
> > > partitionsAndOffsets) for now, and add deleteRecords(Map<TopicPartition,
> > > DeleteRecordsParameter> partitionsAndOffsets) when we need to support
> > core
> > > parameter in the future? By doing this we can make user's experience
> > better
> > > (i.e. not having to instantiate DeleteRecordsParameter) until it is
> > > necessary to be more complicated.
> > >
> > > Dong
> > >
> > > On Wed, Oct 18, 2017 at 4:46 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> > >
> > > > Hi Dong,
> > > >
> > > > I think it makes sense to use the parameters to define the specifics of
> > > the
> > > > request. However, we would probably want to replace the `Long` with a
> > > class
> > > > (similar to `createPartitions`) instead of relying on
> > > > `DeleteRecordsOptions`. The latter is typically used for defining
> > > > additional options, not for defining the core behaviour.
> > > >
> > > > Ismael
> > > >
> > > > On Wed, Oct 18, 2017 at 12:27 AM, Dong Lin <lindon...@gmail.com>
> > wrote:
> > > >
> > > > > Hey Colin,
> > > > >
> > > > > I have also thought about deleteRecordsBeforeOffset so that we can
> > keep
> > > > the
> > > > > name consistent with the existing API in the Scala AdminClient. But
> > > then
> > > > I
> > > > > think it m

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Colin McCabe]

; > > > > > need to specify what ACL permissions are needed to invoke this API.
> > > > > > That should be in the JavaDoc comments as well.  Based on the
> > > previous
> > > > > > discussion, I am assuming that this means DELETE on the TOPIC
> > > resource?
> > > > > > Can you add this to the KIP?
> > > > > >
> > > > > > Right now you have the signature:
> > > > > > > DeleteRecordsResult deleteRecords(Map<TopicPartition, Long>
> > > > > > partitionsAndOffsets)
> > > > > >
> > > > > > Since this function is all about deleting records that come before
> > a
> > > > > > certain offset, how about calling it deleteRecordsBeforeOffset?
> > That
> > > > > > way, if we come up with another way of deleting records in the
> > future
> > > > > > (such as a timestamp or transaction-based way) it will not be
> > > > confusing.
> > > > > >
> > > > > > On Mon, Oct 16, 2017, at 20:50, Becket Qin wrote:
> > > > > > > Hi Paolo,
> > > > > > >
> > > > > > > Thanks for the KIP and sorry for being late on the thread. I am
> > > > > wondering
> > > > > > > what is the KafkaFuture returned by all() call? Should it
> > be
> > > a
> > > > > > > Map<TopicPartition, Long> instead?
> > > > > >
> > > > > > Good point.
> > > > > >
> > > > > > cheers,
> > > > > > Colin
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > > Jiangjie (Becket) QIn
> > > > > > >
> > > > > > > On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <
> > > ppatie...@live.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > >
> > > > > > > > maybe we want to start without the delete records policy for
> > now
> > > > > > waiting
> > > > > > > > for a real needs. So I'm removing it from the KIP.
> > > > > > > >
> > > > > > > > I hope for more comments on this KIP-204 so that we can start a
> > > > vote
> > > > > on
> > > > > > > > Monday.
> > > > > > > >
> > > > > > > >
> > > > > > > > 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: Paolo Patierno <ppatie...@live.com>
> > > > > > > > Sent: Thursday, September 28, 2017 5:56 AM
> > > > > > > > To: dev@kafka.apache.org
> > > > > > > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion
> > > operation
> > > > to
> > > > > > the
> > > > > > > > new Admin Client API
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > >
> > > > > > > > I have just updated the KIP-204 description with the new
> > > > > > > > TopicDeletionPolicy suggested by the KIP-201.
> > > > > > > >
> > > > > > > >
> > > > > > > > Paolo Patierno
> > > > > > > > Senior Software Engineer (IoT) @ Red Hat
> > > > > > > > Microsoft M

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Dong Lin]

is all about deleting records that come before
> a
> > > > > certain offset, how about calling it deleteRecordsBeforeOffset?
> That
> > > > > way, if we come up with another way of deleting records in the
> future
> > > > > (such as a timestamp or transaction-based way) it will not be
> > > confusing.
> > > > >
> > > > > On Mon, Oct 16, 2017, at 20:50, Becket Qin wrote:
> > > > > > Hi Paolo,
> > > > > >
> > > > > > Thanks for the KIP and sorry for being late on the thread. I am
> > > > wondering
> > > > > > what is the KafkaFuture returned by all() call? Should it
> be
> > a
> > > > > > Map<TopicPartition, Long> instead?
> > > > >
> > > > > Good point.
> > > > >
> > > > > cheers,
> > > > > Colin
> > > > >
> > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Jiangjie (Becket) QIn
> > > > > >
> > > > > > On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <
> > ppatie...@live.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > >
> > > > > > > maybe we want to start without the delete records policy for
> now
> > > > > waiting
> > > > > > > for a real needs. So I'm removing it from the KIP.
> > > > > > >
> > > > > > > I hope for more comments on this KIP-204 so that we can start a
> > > vote
> > > > on
> > > > > > > Monday.
> > > > > > >
> > > > > > >
> > > > > > > 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: Paolo Patierno <ppatie...@live.com>
> > > > > > > Sent: Thursday, September 28, 2017 5:56 AM
> > > > > > > To: dev@kafka.apache.org
> > > > > > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion
> > operation
> > > to
> > > > > the
> > > > > > > new Admin Client API
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > >
> > > > > > > I have just updated the KIP-204 description with the new
> > > > > > > TopicDeletionPolicy suggested by the KIP-201.
> > > > > > >
> > > > > > >
> > > > > > > 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: Tuesday, September 26, 2017 4:57 PM
> > > > > > > To: dev@kafka.apache.org
> > > > > > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion
> > operation
> > > to
> > > > > the
> > > > > > > new Admin Client API
> > > > > > >
> > > > > > > Hi Tom,
> > > > > > >
> > > &g

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Ismael Juma]

t; a
> > > > > Map<TopicPartition, Long> instead?
> > > >
> > > > Good point.
> > > >
> > > > cheers,
> > > > Colin
> > > >
> > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Jiangjie (Becket) QIn
> > > > >
> > > > > On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <
> ppatie...@live.com>
> > > > > wrote:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > >
> > > > > > maybe we want to start without the delete records policy for now
> > > > waiting
> > > > > > for a real needs. So I'm removing it from the KIP.
> > > > > >
> > > > > > I hope for more comments on this KIP-204 so that we can start a
> > vote
> > > on
> > > > > > Monday.
> > > > > >
> > > > > >
> > > > > > 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: Paolo Patierno <ppatie...@live.com>
> > > > > > Sent: Thursday, September 28, 2017 5:56 AM
> > > > > > To: dev@kafka.apache.org
> > > > > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion
> operation
> > to
> > > > the
> > > > > > new Admin Client API
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > >
> > > > > > I have just updated the KIP-204 description with the new
> > > > > > TopicDeletionPolicy suggested by the KIP-201.
> > > > > >
> > > > > >
> > > > > > 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: Tuesday, September 26, 2017 4:57 PM
> > > > > > To: dev@kafka.apache.org
> > > > > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion
> operation
> > to
> > > > the
> > > > > > new Admin Client API
> > > > > >
> > > > > > Hi Tom,
> > > > > >
> > > > > > as I said in the KIP-201 discussion I'm ok with having a unique
> > > > > > DeleteTopicPolicy but then it could be useful having more
> > information
> > > > then
> > > > > > just the topic name; partitions and offset for messages deletion
> > > could
> > > > be
> > > > > > useful for a fine grained use cases.
> > > > > >
> > > > > >
> > > > > > 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: Tom

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Dong Lin]

Hey Ismael,

To clarify, I think you are saying that we should replace
"deleteRecords(Map<TopicPartition, Long> partitionsAndOffsets)" with
"deleteRecords(Map<TopicPartition, DeleteRecordsParameter>
partitionsAndOffsets)", where DeleteRecordsParameter should be include a
"Long value", and probably "Boolean isBeforeOrAfter" and "Boolean
isOffsetOrTime" in the future.

I get the point that we want to only include additional parameter
in DeleteRecordsOptions. I just feel it is a bit overkill to have a new
class DeleteRecordsParameter which will only support offset in the near
future. This method signature seems a bit too complicated.

How about we use deleteRecords(Map<TopicPartition, Long>
partitionsAndOffsets) for now, and add deleteRecords(Map<TopicPartition,
DeleteRecordsParameter> partitionsAndOffsets) when we need to support core
parameter in the future? By doing this we can make user's experience better
(i.e. not having to instantiate DeleteRecordsParameter) until it is
necessary to be more complicated.

Dong

On Wed, Oct 18, 2017 at 4:46 AM, Ismael Juma <ism...@juma.me.uk> wrote:

> Hi Dong,
>
> I think it makes sense to use the parameters to define the specifics of the
> request. However, we would probably want to replace the `Long` with a class
> (similar to `createPartitions`) instead of relying on
> `DeleteRecordsOptions`. The latter is typically used for defining
> additional options, not for defining the core behaviour.
>
> Ismael
>
> On Wed, Oct 18, 2017 at 12:27 AM, Dong Lin <lindon...@gmail.com> wrote:
>
> > Hey Colin,
> >
> > I have also thought about deleteRecordsBeforeOffset so that we can keep
> the
> > name consistent with the existing API in the Scala AdminClient. But then
> I
> > think it may be better to be able to specify in DeleteRecordsOptions
> > whether the deletion is before/after timestamp or offset. By doing this
> we
> > have one API rather than four API in Java AdminClient going forward. What
> > do you think?
> >
> > Thanks,
> > Dong
> >
> > On Tue, Oct 17, 2017 at 11:35 AM, Colin McCabe <cmcc...@apache.org>
> wrote:
> >
> > > Hi Paolo,
> > >
> > > This is a nice improvement.
> > >
> > > I agree that the discussion of creating a DeleteTopicPolicy can wait
> > > until later.  Perhaps we can do it in a follow-on KIP.  However, we do
> > > need to specify what ACL permissions are needed to invoke this API.
> > > That should be in the JavaDoc comments as well.  Based on the previous
> > > discussion, I am assuming that this means DELETE on the TOPIC resource?
> > > Can you add this to the KIP?
> > >
> > > Right now you have the signature:
> > > > DeleteRecordsResult deleteRecords(Map<TopicPartition, Long>
> > > partitionsAndOffsets)
> > >
> > > Since this function is all about deleting records that come before a
> > > certain offset, how about calling it deleteRecordsBeforeOffset?  That
> > > way, if we come up with another way of deleting records in the future
> > > (such as a timestamp or transaction-based way) it will not be
> confusing.
> > >
> > > On Mon, Oct 16, 2017, at 20:50, Becket Qin wrote:
> > > > Hi Paolo,
> > > >
> > > > Thanks for the KIP and sorry for being late on the thread. I am
> > wondering
> > > > what is the KafkaFuture returned by all() call? Should it be a
> > > > Map<TopicPartition, Long> instead?
> > >
> > > Good point.
> > >
> > > cheers,
> > > Colin
> > >
> > >
> > > >
> > > > Thanks,
> > > >
> > > > Jiangjie (Becket) QIn
> > > >
> > > > On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <ppatie...@live.com>
> > > > wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > >
> > > > > maybe we want to start without the delete records policy for now
> > > waiting
> > > > > for a real needs. So I'm removing it from the KIP.
> > > > >
> > > > > I hope for more comments on this KIP-204 so that we can start a
> vote
> > on
> > > > > Monday.
> > > > >
> > > > >
> > > > > Thanks.
> > > > >
> > > > >
> > > > > Paolo Patierno
> > > > > Senior Software Engineer (IoT) @ Red Hat
> > > > > Microsoft MVP on Azure & IoT
> > > > > Microsoft Azure Advis

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Ismael Juma]

Hi Dong,

I think it makes sense to use the parameters to define the specifics of the
request. However, we would probably want to replace the `Long` with a class
(similar to `createPartitions`) instead of relying on
`DeleteRecordsOptions`. The latter is typically used for defining
additional options, not for defining the core behaviour.

Ismael

On Wed, Oct 18, 2017 at 12:27 AM, Dong Lin <lindon...@gmail.com> wrote:

> Hey Colin,
>
> I have also thought about deleteRecordsBeforeOffset so that we can keep the
> name consistent with the existing API in the Scala AdminClient. But then I
> think it may be better to be able to specify in DeleteRecordsOptions
> whether the deletion is before/after timestamp or offset. By doing this we
> have one API rather than four API in Java AdminClient going forward. What
> do you think?
>
> Thanks,
> Dong
>
> On Tue, Oct 17, 2017 at 11:35 AM, Colin McCabe <cmcc...@apache.org> wrote:
>
> > Hi Paolo,
> >
> > This is a nice improvement.
> >
> > I agree that the discussion of creating a DeleteTopicPolicy can wait
> > until later.  Perhaps we can do it in a follow-on KIP.  However, we do
> > need to specify what ACL permissions are needed to invoke this API.
> > That should be in the JavaDoc comments as well.  Based on the previous
> > discussion, I am assuming that this means DELETE on the TOPIC resource?
> > Can you add this to the KIP?
> >
> > Right now you have the signature:
> > > DeleteRecordsResult deleteRecords(Map<TopicPartition, Long>
> > partitionsAndOffsets)
> >
> > Since this function is all about deleting records that come before a
> > certain offset, how about calling it deleteRecordsBeforeOffset?  That
> > way, if we come up with another way of deleting records in the future
> > (such as a timestamp or transaction-based way) it will not be confusing.
> >
> > On Mon, Oct 16, 2017, at 20:50, Becket Qin wrote:
> > > Hi Paolo,
> > >
> > > Thanks for the KIP and sorry for being late on the thread. I am
> wondering
> > > what is the KafkaFuture returned by all() call? Should it be a
> > > Map<TopicPartition, Long> instead?
> >
> > Good point.
> >
> > cheers,
> > Colin
> >
> >
> > >
> > > Thanks,
> > >
> > > Jiangjie (Becket) QIn
> > >
> > > On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <ppatie...@live.com>
> > > wrote:
> > >
> > > > Hi,
> > > >
> > > >
> > > > maybe we want to start without the delete records policy for now
> > waiting
> > > > for a real needs. So I'm removing it from the KIP.
> > > >
> > > > I hope for more comments on this KIP-204 so that we can start a vote
> on
> > > > Monday.
> > > >
> > > >
> > > > 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: Paolo Patierno <ppatie...@live.com>
> > > > Sent: Thursday, September 28, 2017 5:56 AM
> > > > To: dev@kafka.apache.org
> > > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to
> > the
> > > > new Admin Client API
> > > >
> > > > Hi,
> > > >
> > > >
> > > > I have just updated the KIP-204 description with the new
> > > > TopicDeletionPolicy suggested by the KIP-201.
> > > >
> > > >
> > > > 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: Tuesday, September 26, 2017

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Dong Lin]

Hey Colin,

I have also thought about deleteRecordsBeforeOffset so that we can keep the
name consistent with the existing API in the Scala AdminClient. But then I
think it may be better to be able to specify in DeleteRecordsOptions
whether the deletion is before/after timestamp or offset. By doing this we
have one API rather than four API in Java AdminClient going forward. What
do you think?

Thanks,
Dong

On Tue, Oct 17, 2017 at 11:35 AM, Colin McCabe <cmcc...@apache.org> wrote:

> Hi Paolo,
>
> This is a nice improvement.
>
> I agree that the discussion of creating a DeleteTopicPolicy can wait
> until later.  Perhaps we can do it in a follow-on KIP.  However, we do
> need to specify what ACL permissions are needed to invoke this API.
> That should be in the JavaDoc comments as well.  Based on the previous
> discussion, I am assuming that this means DELETE on the TOPIC resource?
> Can you add this to the KIP?
>
> Right now you have the signature:
> > DeleteRecordsResult deleteRecords(Map<TopicPartition, Long>
> partitionsAndOffsets)
>
> Since this function is all about deleting records that come before a
> certain offset, how about calling it deleteRecordsBeforeOffset?  That
> way, if we come up with another way of deleting records in the future
> (such as a timestamp or transaction-based way) it will not be confusing.
>
> On Mon, Oct 16, 2017, at 20:50, Becket Qin wrote:
> > Hi Paolo,
> >
> > Thanks for the KIP and sorry for being late on the thread. I am wondering
> > what is the KafkaFuture returned by all() call? Should it be a
> > Map<TopicPartition, Long> instead?
>
> Good point.
>
> cheers,
> Colin
>
>
> >
> > Thanks,
> >
> > Jiangjie (Becket) QIn
> >
> > On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <ppatie...@live.com>
> > wrote:
> >
> > > Hi,
> > >
> > >
> > > maybe we want to start without the delete records policy for now
> waiting
> > > for a real needs. So I'm removing it from the KIP.
> > >
> > > I hope for more comments on this KIP-204 so that we can start a vote on
> > > Monday.
> > >
> > >
> > > 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: Paolo Patierno <ppatie...@live.com>
> > > Sent: Thursday, September 28, 2017 5:56 AM
> > > To: dev@kafka.apache.org
> > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to
> the
> > > new Admin Client API
> > >
> > > Hi,
> > >
> > >
> > > I have just updated the KIP-204 description with the new
> > > TopicDeletionPolicy suggested by the KIP-201.
> > >
> > >
> > > 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: Tuesday, September 26, 2017 4:57 PM
> > > To: dev@kafka.apache.org
> > > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to
> the
> > > new Admin Client API
> > >
> > > Hi Tom,
> > >
> > > as I said in the KIP-201 discussion I'm ok with having a unique
> > > DeleteTopicPolicy but then it could be useful having more information
> then
> > > just the topic name; partitions and offset for messages deletion could
> be
> > > useful for a fine grained use cases.
> > >
> > >
> > > 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

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Colin McCabe]

Hi Paolo,

This is a nice improvement.

I agree that the discussion of creating a DeleteTopicPolicy can wait
until later.  Perhaps we can do it in a follow-on KIP.  However, we do
need to specify what ACL permissions are needed to invoke this API. 
That should be in the JavaDoc comments as well.  Based on the previous
discussion, I am assuming that this means DELETE on the TOPIC resource? 
Can you add this to the KIP?

Right now you have the signature:
> DeleteRecordsResult deleteRecords(Map<TopicPartition, Long> 
> partitionsAndOffsets)

Since this function is all about deleting records that come before a
certain offset, how about calling it deleteRecordsBeforeOffset?  That
way, if we come up with another way of deleting records in the future
(such as a timestamp or transaction-based way) it will not be confusing.

On Mon, Oct 16, 2017, at 20:50, Becket Qin wrote:
> Hi Paolo,
> 
> Thanks for the KIP and sorry for being late on the thread. I am wondering
> what is the KafkaFuture returned by all() call? Should it be a
> Map<TopicPartition, Long> instead?

Good point.

cheers,
Colin


> 
> Thanks,
> 
> Jiangjie (Becket) QIn
> 
> On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <ppatie...@live.com>
> wrote:
> 
> > Hi,
> >
> >
> > maybe we want to start without the delete records policy for now waiting
> > for a real needs. So I'm removing it from the KIP.
> >
> > I hope for more comments on this KIP-204 so that we can start a vote on
> > Monday.
> >
> >
> > 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: Paolo Patierno <ppatie...@live.com>
> > Sent: Thursday, September 28, 2017 5:56 AM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > Hi,
> >
> >
> > I have just updated the KIP-204 description with the new
> > TopicDeletionPolicy suggested by the KIP-201.
> >
> >
> > 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: Tuesday, September 26, 2017 4:57 PM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > Hi Tom,
> >
> > as I said in the KIP-201 discussion I'm ok with having a unique
> > DeleteTopicPolicy but then it could be useful having more information then
> > just the topic name; partitions and offset for messages deletion could be
> > useful for a fine grained use cases.
> >
> >
> > 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: Tom Bentley <t.j.bent...@gmail.com>
> > Sent: Tuesday, September 26, 2017 4:32 PM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> > new Admin Client API
> >
> > Hi Paolo,
> >
> > I guess a RecordDeletionPolicy should work at the partition level, whereas
> > the TopicDeletionPolicy should work at the topic level. But then we run
> > into a similar situation as described in the motivation for KIP-201, where
> > the administrator might have to write+configure two policies in order to
> > express their intended rules. For example, it's no good preventing people
> > from deleting topics if they can delete all the messages in those topics,
> > or vice versa.
> >
> > On that reasoning, perhaps there should be a single policy interface
> > covering topic deletion and 

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Becket Qin]

Hi Paolo,

Thanks for the KIP and sorry for being late on the thread. I am wondering
what is the KafkaFuture returned by all() call? Should it be a
Map<TopicPartition, Long> instead?

Thanks,

Jiangjie (Becket) QIn

On Thu, Sep 28, 2017 at 3:48 AM, Paolo Patierno <ppatie...@live.com> wrote:

> Hi,
>
>
> maybe we want to start without the delete records policy for now waiting
> for a real needs. So I'm removing it from the KIP.
>
> I hope for more comments on this KIP-204 so that we can start a vote on
> Monday.
>
>
> 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: Paolo Patierno <ppatie...@live.com>
> Sent: Thursday, September 28, 2017 5:56 AM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi,
>
>
> I have just updated the KIP-204 description with the new
> TopicDeletionPolicy suggested by the KIP-201.
>
>
> 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: Tuesday, September 26, 2017 4:57 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Tom,
>
> as I said in the KIP-201 discussion I'm ok with having a unique
> DeleteTopicPolicy but then it could be useful having more information then
> just the topic name; partitions and offset for messages deletion could be
> useful for a fine grained use cases.
>
>
> 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: Tom Bentley <t.j.bent...@gmail.com>
> Sent: Tuesday, September 26, 2017 4:32 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Paolo,
>
> I guess a RecordDeletionPolicy should work at the partition level, whereas
> the TopicDeletionPolicy should work at the topic level. But then we run
> into a similar situation as described in the motivation for KIP-201, where
> the administrator might have to write+configure two policies in order to
> express their intended rules. For example, it's no good preventing people
> from deleting topics if they can delete all the messages in those topics,
> or vice versa.
>
> On that reasoning, perhaps there should be a single policy interface
> covering topic deletion and message deletion. Alternatively, the topic
> deletion API could also invoke the record deletion policy (before the topic
> deletion policy I mean). But the former would be more consistent with
> what's proposed in KIP-201.
>
> Wdyt? I can add this to KIP-201 if you want.
>
> Cheers,
>
> Tom
>
>
>
>
>
> On 26 September 2017 at 17:01, Paolo Patierno <ppatie...@live.com> wrote:
>
> > Hi Tom,
> >
> > I think that we could live with the current authorizer based on delete
> > topic (for both, deleting messages and topic as a whole) but then the
> > RecordsDeletePolicy could be even more fine grained giving the
> possibility
> > to avoid deleting messages for specific partitions (inside the topic) and
> > starting from a specific offset.
> >
> > I could think on some users solutions where they know exactly what the
> > partitions means inside of a specific topic (because they are using a
> > custom partitioner on the producer side) so they know what kind of
> messages
> > are inside a partition allowing to delete them but not the other.
> >
> > In such a policy a user could also check the timestamp related to the
> > offset for allowing or not deletion on time base.
> >
> >
> > Wdyt ?
> >
> >
> > Paolo Patierno
> > Senior Software Eng

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi,


maybe we want to start without the delete records policy for now waiting for a 
real needs. So I'm removing it from the KIP.

I hope for more comments on this KIP-204 so that we can start a vote on Monday.


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: Paolo Patierno <ppatie...@live.com>
Sent: Thursday, September 28, 2017 5:56 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi,


I have just updated the KIP-204 description with the new TopicDeletionPolicy 
suggested by the KIP-201.


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: Tuesday, September 26, 2017 4:57 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Tom,

as I said in the KIP-201 discussion I'm ok with having a unique 
DeleteTopicPolicy but then it could be useful having more information then just 
the topic name; partitions and offset for messages deletion could be useful for 
a fine grained use cases.


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: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 26, 2017 4:32 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

I guess a RecordDeletionPolicy should work at the partition level, whereas
the TopicDeletionPolicy should work at the topic level. But then we run
into a similar situation as described in the motivation for KIP-201, where
the administrator might have to write+configure two policies in order to
express their intended rules. For example, it's no good preventing people
from deleting topics if they can delete all the messages in those topics,
or vice versa.

On that reasoning, perhaps there should be a single policy interface
covering topic deletion and message deletion. Alternatively, the topic
deletion API could also invoke the record deletion policy (before the topic
deletion policy I mean). But the former would be more consistent with
what's proposed in KIP-201.

Wdyt? I can add this to KIP-201 if you want.

Cheers,

Tom





On 26 September 2017 at 17:01, Paolo Patierno <ppatie...@live.com> wrote:

> Hi Tom,
>
> I think that we could live with the current authorizer based on delete
> topic (for both, deleting messages and topic as a whole) but then the
> RecordsDeletePolicy could be even more fine grained giving the possibility
> to avoid deleting messages for specific partitions (inside the topic) and
> starting from a specific offset.
>
> I could think on some users solutions where they know exactly what the
> partitions means inside of a specific topic (because they are using a
> custom partitioner on the producer side) so they know what kind of messages
> are inside a partition allowing to delete them but not the other.
>
> In such a policy a user could also check the timestamp related to the
> offset for allowing or not deletion on time base.
>
>
> Wdyt ?
>
>
> 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: Tom Bentley <t.j.bent...@gmail.com>
> Sent: Tuesday, September 26, 2017 2:55 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Edoardo and Paolo,
>
>
> On 26 September 2017 at 14:10, Paolo Patierno <ppatie...@live.com> wrote:
>
> > What could be useful use cases for having a RecordsDeletePolicy ? Records
> > can't be deleted for a topic name ? Starting from a specific offset ?
> >
>
> I can imagine some

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi,


I have just updated the KIP-204 description with the new TopicDeletionPolicy 
suggested by the KIP-201.


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: Tuesday, September 26, 2017 4:57 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Tom,

as I said in the KIP-201 discussion I'm ok with having a unique 
DeleteTopicPolicy but then it could be useful having more information then just 
the topic name; partitions and offset for messages deletion could be useful for 
a fine grained use cases.


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: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 26, 2017 4:32 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

I guess a RecordDeletionPolicy should work at the partition level, whereas
the TopicDeletionPolicy should work at the topic level. But then we run
into a similar situation as described in the motivation for KIP-201, where
the administrator might have to write+configure two policies in order to
express their intended rules. For example, it's no good preventing people
from deleting topics if they can delete all the messages in those topics,
or vice versa.

On that reasoning, perhaps there should be a single policy interface
covering topic deletion and message deletion. Alternatively, the topic
deletion API could also invoke the record deletion policy (before the topic
deletion policy I mean). But the former would be more consistent with
what's proposed in KIP-201.

Wdyt? I can add this to KIP-201 if you want.

Cheers,

Tom





On 26 September 2017 at 17:01, Paolo Patierno <ppatie...@live.com> wrote:

> Hi Tom,
>
> I think that we could live with the current authorizer based on delete
> topic (for both, deleting messages and topic as a whole) but then the
> RecordsDeletePolicy could be even more fine grained giving the possibility
> to avoid deleting messages for specific partitions (inside the topic) and
> starting from a specific offset.
>
> I could think on some users solutions where they know exactly what the
> partitions means inside of a specific topic (because they are using a
> custom partitioner on the producer side) so they know what kind of messages
> are inside a partition allowing to delete them but not the other.
>
> In such a policy a user could also check the timestamp related to the
> offset for allowing or not deletion on time base.
>
>
> Wdyt ?
>
>
> 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: Tom Bentley <t.j.bent...@gmail.com>
> Sent: Tuesday, September 26, 2017 2:55 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Edoardo and Paolo,
>
>
> On 26 September 2017 at 14:10, Paolo Patierno <ppatie...@live.com> wrote:
>
> > What could be useful use cases for having a RecordsDeletePolicy ? Records
> > can't be deleted for a topic name ? Starting from a specific offset ?
> >
>
> I can imagine some users wanting to prohibit using this API completely.
> Maybe others divide up the topic namespace according to some scheme, and so
> it would be allowed for some topics, but not for others based on the name.
> Both these could be done using authz, but would be much simpler to express
> using a policy.
>
> Since both deleting messages and deleting topics are authorized using
> delete operation on the topic, using policies it would be possible to allow
> deleting messages from a topic, but not deleting the topic itself.
>
>
> On 26 September 2017 at 15:27, Edoardo Comar <eco...@uk.ibm.com> wrote:
>
> >
> > Our KIP-170 did indeed suggest a TopicDeletePolicy - but, as said, for
> our
> > intent a

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Tom,

as I said in the KIP-201 discussion I'm ok with having a unique 
DeleteTopicPolicy but then it could be useful having more information then just 
the topic name; partitions and offset for messages deletion could be useful for 
a fine grained use cases.


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: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 26, 2017 4:32 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

I guess a RecordDeletionPolicy should work at the partition level, whereas
the TopicDeletionPolicy should work at the topic level. But then we run
into a similar situation as described in the motivation for KIP-201, where
the administrator might have to write+configure two policies in order to
express their intended rules. For example, it's no good preventing people
from deleting topics if they can delete all the messages in those topics,
or vice versa.

On that reasoning, perhaps there should be a single policy interface
covering topic deletion and message deletion. Alternatively, the topic
deletion API could also invoke the record deletion policy (before the topic
deletion policy I mean). But the former would be more consistent with
what's proposed in KIP-201.

Wdyt? I can add this to KIP-201 if you want.

Cheers,

Tom





On 26 September 2017 at 17:01, Paolo Patierno <ppatie...@live.com> wrote:

> Hi Tom,
>
> I think that we could live with the current authorizer based on delete
> topic (for both, deleting messages and topic as a whole) but then the
> RecordsDeletePolicy could be even more fine grained giving the possibility
> to avoid deleting messages for specific partitions (inside the topic) and
> starting from a specific offset.
>
> I could think on some users solutions where they know exactly what the
> partitions means inside of a specific topic (because they are using a
> custom partitioner on the producer side) so they know what kind of messages
> are inside a partition allowing to delete them but not the other.
>
> In such a policy a user could also check the timestamp related to the
> offset for allowing or not deletion on time base.
>
>
> Wdyt ?
>
>
> 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: Tom Bentley <t.j.bent...@gmail.com>
> Sent: Tuesday, September 26, 2017 2:55 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Edoardo and Paolo,
>
>
> On 26 September 2017 at 14:10, Paolo Patierno <ppatie...@live.com> wrote:
>
> > What could be useful use cases for having a RecordsDeletePolicy ? Records
> > can't be deleted for a topic name ? Starting from a specific offset ?
> >
>
> I can imagine some users wanting to prohibit using this API completely.
> Maybe others divide up the topic namespace according to some scheme, and so
> it would be allowed for some topics, but not for others based on the name.
> Both these could be done using authz, but would be much simpler to express
> using a policy.
>
> Since both deleting messages and deleting topics are authorized using
> delete operation on the topic, using policies it would be possible to allow
> deleting messages from a topic, but not deleting the topic itself.
>
>
> On 26 September 2017 at 15:27, Edoardo Comar <eco...@uk.ibm.com> wrote:
>
> >
> > Our KIP-170 did indeed suggest a TopicDeletePolicy - but, as said, for
> our
> > intent an Authorizer implementation will be usable instead,
> >
>
> I guess authorization in the most general sense encompass es both the
> ACL-based authorization inherent in Authorizer and the various
> operation-specific *Policies. But they're not the same. The Policies are
> about deciding on *what* is requested, and the Authorizer is about making a
> decision purely on *who* is making the request. It's quite legitimate to
> want to use both, or just one or the other.
>

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Tom Bentley]

Hi Paolo,

I guess a RecordDeletionPolicy should work at the partition level, whereas
the TopicDeletionPolicy should work at the topic level. But then we run
into a similar situation as described in the motivation for KIP-201, where
the administrator might have to write+configure two policies in order to
express their intended rules. For example, it's no good preventing people
from deleting topics if they can delete all the messages in those topics,
or vice versa.

On that reasoning, perhaps there should be a single policy interface
covering topic deletion and message deletion. Alternatively, the topic
deletion API could also invoke the record deletion policy (before the topic
deletion policy I mean). But the former would be more consistent with
what's proposed in KIP-201.

Wdyt? I can add this to KIP-201 if you want.

Cheers,

Tom





On 26 September 2017 at 17:01, Paolo Patierno <ppatie...@live.com> wrote:

> Hi Tom,
>
> I think that we could live with the current authorizer based on delete
> topic (for both, deleting messages and topic as a whole) but then the
> RecordsDeletePolicy could be even more fine grained giving the possibility
> to avoid deleting messages for specific partitions (inside the topic) and
> starting from a specific offset.
>
> I could think on some users solutions where they know exactly what the
> partitions means inside of a specific topic (because they are using a
> custom partitioner on the producer side) so they know what kind of messages
> are inside a partition allowing to delete them but not the other.
>
> In such a policy a user could also check the timestamp related to the
> offset for allowing or not deletion on time base.
>
>
> Wdyt ?
>
>
> 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: Tom Bentley <t.j.bent...@gmail.com>
> Sent: Tuesday, September 26, 2017 2:55 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
> new Admin Client API
>
> Hi Edoardo and Paolo,
>
>
> On 26 September 2017 at 14:10, Paolo Patierno <ppatie...@live.com> wrote:
>
> > What could be useful use cases for having a RecordsDeletePolicy ? Records
> > can't be deleted for a topic name ? Starting from a specific offset ?
> >
>
> I can imagine some users wanting to prohibit using this API completely.
> Maybe others divide up the topic namespace according to some scheme, and so
> it would be allowed for some topics, but not for others based on the name.
> Both these could be done using authz, but would be much simpler to express
> using a policy.
>
> Since both deleting messages and deleting topics are authorized using
> delete operation on the topic, using policies it would be possible to allow
> deleting messages from a topic, but not deleting the topic itself.
>
>
> On 26 September 2017 at 15:27, Edoardo Comar <eco...@uk.ibm.com> wrote:
>
> >
> > Our KIP-170 did indeed suggest a TopicDeletePolicy - but, as said, for
> our
> > intent an Authorizer implementation will be usable instead,
> >
>
> I guess authorization in the most general sense encompass es both the
> ACL-based authorization inherent in Authorizer and the various
> operation-specific *Policies. But they're not the same. The Policies are
> about deciding on *what* is requested, and the Authorizer is about making a
> decision purely on *who* is making the request. It's quite legitimate to
> want to use both, or just one or the other.
>

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Tom,

I think that we could live with the current authorizer based on delete topic 
(for both, deleting messages and topic as a whole) but then the 
RecordsDeletePolicy could be even more fine grained giving the possibility to 
avoid deleting messages for specific partitions (inside the topic) and starting 
from a specific offset.

I could think on some users solutions where they know exactly what the 
partitions means inside of a specific topic (because they are using a custom 
partitioner on the producer side) so they know what kind of messages are inside 
a partition allowing to delete them but not the other.

In such a policy a user could also check the timestamp related to the offset 
for allowing or not deletion on time base.


Wdyt ?


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: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 26, 2017 2:55 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Edoardo and Paolo,


On 26 September 2017 at 14:10, Paolo Patierno <ppatie...@live.com> wrote:

> What could be useful use cases for having a RecordsDeletePolicy ? Records
> can't be deleted for a topic name ? Starting from a specific offset ?
>

I can imagine some users wanting to prohibit using this API completely.
Maybe others divide up the topic namespace according to some scheme, and so
it would be allowed for some topics, but not for others based on the name.
Both these could be done using authz, but would be much simpler to express
using a policy.

Since both deleting messages and deleting topics are authorized using
delete operation on the topic, using policies it would be possible to allow
deleting messages from a topic, but not deleting the topic itself.


On 26 September 2017 at 15:27, Edoardo Comar <eco...@uk.ibm.com> wrote:

>
> Our KIP-170 did indeed suggest a TopicDeletePolicy - but, as said, for our
> intent an Authorizer implementation will be usable instead,
>

I guess authorization in the most general sense encompass es both the
ACL-based authorization inherent in Authorizer and the various
operation-specific *Policies. But they're not the same. The Policies are
about deciding on *what* is requested, and the Authorizer is about making a
decision purely on *who* is making the request. It's quite legitimate to
want to use both, or just one or the other.

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Tom Bentley]

Hi Edoardo and Paolo,


On 26 September 2017 at 14:10, Paolo Patierno  wrote:

> What could be useful use cases for having a RecordsDeletePolicy ? Records
> can't be deleted for a topic name ? Starting from a specific offset ?
>

I can imagine some users wanting to prohibit using this API completely.
Maybe others divide up the topic namespace according to some scheme, and so
it would be allowed for some topics, but not for others based on the name.
Both these could be done using authz, but would be much simpler to express
using a policy.

Since both deleting messages and deleting topics are authorized using
delete operation on the topic, using policies it would be possible to allow
deleting messages from a topic, but not deleting the topic itself.


On 26 September 2017 at 15:27, Edoardo Comar  wrote:

>
> Our KIP-170 did indeed suggest a TopicDeletePolicy - but, as said, for our
> intent an Authorizer implementation will be usable instead,
>

I guess authorization in the most general sense encompass es both the
ACL-based authorization inherent in Authorizer and the various
operation-specific *Policies. But they're not the same. The Policies are
about deciding on *what* is requested, and the Authorizer is about making a
decision purely on *who* is making the request. It's quite legitimate to
want to use both, or just one or the other.

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Edoardo,


I was referring to the KIP-107 where the delete records operation is coming 
with the authorizer I mentioned. You are referring to KIP-170 ... same digits, 
inverse order ! Sorry for that ;)


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: Tuesday, September 26, 2017 2:12 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Edoardo,

reading the KIP-170 at this point :

3) API Authorization

Given the potential damage that can be caused if this API is used by mistake, 
it is important that we limit its usage to only authorized users. For this 
matter, we can take advantage of the existing authorization framework 
implemented in 
KIP-11<https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface>.
 deleteDataBefore() will have the same authorization setting as deleteTopic(). 
Its operation type is be DELETE and its resource type is TOPIC.


it seems to me that the KIP doesn't suggest a policy as you are saying but 
using the authorizer mechanism with operation = DELETE and resource = TOPIC.

Is my understanding right ?


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: Edoardo Comar <eco...@uk.ibm.com>
Sent: Tuesday, September 26, 2017 1:59 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

our the motivation for suggesting a delete policy as described in KIP-170
was to avoid deletion of topics that have a specific usage (essential to
other services).
As you say, it would be entirely replaceable by an Authorizer
implementation that performs the equivalent check.

thanks
Edo
--

Edoardo Comar

IBM Message Hub

IBM UK Ltd, Hursley Park, SO21 2JN



From:   Paolo Patierno <ppatie...@live.com>
To: "dev@kafka.apache.org" <dev@kafka.apache.org>
Date:   26/09/2017 14:10
Subject:    Re: [DISCUSS] KIP-204 : adding records deletion operation
to the new Admin Client API



Hi Tom,


the KIP-170 doesn't propose to use a TopicDeletePolicy as policy classes
are meant to be.

It's referring to the authorization interface (KIP-11<
https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D11-2B-2D-2BAuthorization-2BInterface=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=NQw-765HFBIbCYIZLpYIJrvCaZ4-Ml0L1e4z4wkx8fg=
>) with operation = DELETE and resource = TOPIC.

You know that when a request comes in there is the "authorizer" first
which is based on KIP-11 and then the operation is called and then you
could have policy.

I.e. for create topics, first the "authorizer" is called in the KafkaApis
(operation = CREATE, resource = TOPIC) and then the CreateTopicPolicy is
applied in the AdminManager.


In any case what KIP-170 is proposing is not good. Maybe we could have
operation = DELETE and a new resource = MESSAGE/RECORD.

What could be useful use cases for having a RecordsDeletePolicy ? Records
can't be deleted for a topic name ? Starting from a specific offset ?


Paolo Patierno
Senior Software Engineer (IoT) @ Red Hat
Microsoft MVP on Azure & IoT
Microsoft Azure Advisor

Twitter : @ppatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter.com_ppatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=XDNIqoF_Lv9mDGsBvFaRn8bazYxBs3a4lWKrekvRxrw=
>
Linkedin : paolopatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__it.linkedin.com_in_paolopatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=6dww9uXw2L0IZ3JKqvoxImEA07L5d7ORCtWi3_wxwcw=
>
Blog : DevExperience<
https://urldefense.proofpoint.com/v2/url?u=http-3A__paolopatierno.wordpress.com_=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=RcYQ4n1Fcl2vHgy2Aj_J9wUO3ZUaFgl8vCZ4slCD5ro=
>


____
From: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 19, 2017 8:55 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 :

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Edoardo Comar]

Paolo,
the link you posted is to KIP-11, and I can't find the text you quoted.

Our KIP-170 did indeed suggest a TopicDeletePolicy - but, as said, for our 
intent an Authorizer implementation will be usable instead,

thanks
--

Edoardo Comar

IBM Message Hub

IBM UK Ltd, Hursley Park, SO21 2JN



From:   Paolo Patierno <ppatie...@live.com>
To: "dev@kafka.apache.org" <dev@kafka.apache.org>
Date:   26/09/2017 15:12
Subject:        Re: [DISCUSS] KIP-204 : adding records deletion operation 
to the new Admin Client API



Hi Edoardo,

reading the KIP-170 at this point :

3) API Authorization

Given the potential damage that can be caused if this API is used by 
mistake, it is important that we limit its usage to only authorized users. 
For this matter, we can take advantage of the existing authorization 
framework implemented in KIP-11<
https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D11-2B-2D-2BAuthorization-2BInterface=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=_bBU3gCyRRvtPMETFlj42fUK_5k1BPbbuAd4ss0Q3EY=rnXlM4Qr3HNrHCQ2ylnl0oVwD66FkfeeOXuQnJ2HPeo=
 
>. deleteDataBefore() will have the same authorization setting as 
deleteTopic(). Its operation type is be DELETE and its resource type is 
TOPIC.


it seems to me that the KIP doesn't suggest a policy as you are saying but 
using the authorizer mechanism with operation = DELETE and resource = 
TOPIC.

Is my understanding right ?


Paolo Patierno
Senior Software Engineer (IoT) @ Red Hat
Microsoft MVP on Azure & IoT
Microsoft Azure Advisor

Twitter : @ppatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter.com_ppatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=_bBU3gCyRRvtPMETFlj42fUK_5k1BPbbuAd4ss0Q3EY=7erpxl-Jff3s9-dWHT45yZPeSfwRvNF1p8STjKtdhbw=
 
>
Linkedin : paolopatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__it.linkedin.com_in_paolopatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=_bBU3gCyRRvtPMETFlj42fUK_5k1BPbbuAd4ss0Q3EY=6YTI7dCn81XZCfx3kJCdhrFDlqujd8GYJyBwbKOYsZU=
 
>
Blog : DevExperience<
https://urldefense.proofpoint.com/v2/url?u=http-3A__paolopatierno.wordpress.com_=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=_bBU3gCyRRvtPMETFlj42fUK_5k1BPbbuAd4ss0Q3EY=QL003tyI8pPos0HCHpKsjsXQ-hfsJeHd_JrglBE-9q4=
 
>



From: Edoardo Comar <eco...@uk.ibm.com>
Sent: Tuesday, September 26, 2017 1:59 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the 
new Admin Client API

Hi Paolo,

our the motivation for suggesting a delete policy as described in KIP-170
was to avoid deletion of topics that have a specific usage (essential to
other services).
As you say, it would be entirely replaceable by an Authorizer
implementation that performs the equivalent check.

thanks
Edo
--

Edoardo Comar

IBM Message Hub

IBM UK Ltd, Hursley Park, SO21 2JN



From:   Paolo Patierno <ppatie...@live.com>
To: "dev@kafka.apache.org" <dev@kafka.apache.org>
Date:   26/09/2017 14:10
Subject:    Re: [DISCUSS] KIP-204 : adding records deletion operation
to the new Admin Client API



Hi Tom,


the KIP-170 doesn't propose to use a TopicDeletePolicy as policy classes
are meant to be.

It's referring to the authorization interface (KIP-11<
https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D11-2B-2D-2BAuthorization-2BInterface=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=NQw-765HFBIbCYIZLpYIJrvCaZ4-Ml0L1e4z4wkx8fg=
>) with operation = DELETE and resource = TOPIC.

You know that when a request comes in there is the "authorizer" first
which is based on KIP-11 and then the operation is called and then you
could have policy.

I.e. for create topics, first the "authorizer" is called in the KafkaApis
(operation = CREATE, resource = TOPIC) and then the CreateTopicPolicy is
applied in the AdminManager.


In any case what KIP-170 is proposing is not good. Maybe we could have
operation = DELETE and a new resource = MESSAGE/RECORD.

What could be useful use cases for having a RecordsDeletePolicy ? Records
can't be deleted for a topic name ? Starting from a specific offset ?


Paolo Patierno
Senior Software Engineer (IoT) @ Red Hat
Microsoft MVP on Azure & IoT
Microsoft Azure Advisor

Twitter : @ppatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter.com_ppatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=XDNIqoF_Lv9mDGsBvFaRn8bazYxBs3a4lWKrekvRxrw=
>
Linkedin : paolopatierno&l

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Edoardo,

reading the KIP-170 at this point :

3) API Authorization

Given the potential damage that can be caused if this API is used by mistake, 
it is important that we limit its usage to only authorized users. For this 
matter, we can take advantage of the existing authorization framework 
implemented in 
KIP-11<https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface>.
 deleteDataBefore() will have the same authorization setting as deleteTopic(). 
Its operation type is be DELETE and its resource type is TOPIC.


it seems to me that the KIP doesn't suggest a policy as you are saying but 
using the authorizer mechanism with operation = DELETE and resource = TOPIC.

Is my understanding right ?


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: Edoardo Comar <eco...@uk.ibm.com>
Sent: Tuesday, September 26, 2017 1:59 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

our the motivation for suggesting a delete policy as described in KIP-170
was to avoid deletion of topics that have a specific usage (essential to
other services).
As you say, it would be entirely replaceable by an Authorizer
implementation that performs the equivalent check.

thanks
Edo
--

Edoardo Comar

IBM Message Hub

IBM UK Ltd, Hursley Park, SO21 2JN



From:   Paolo Patierno <ppatie...@live.com>
To: "dev@kafka.apache.org" <dev@kafka.apache.org>
Date:   26/09/2017 14:10
Subject:Re: [DISCUSS] KIP-204 : adding records deletion operation
to the new Admin Client API



Hi Tom,


the KIP-170 doesn't propose to use a TopicDeletePolicy as policy classes
are meant to be.

It's referring to the authorization interface (KIP-11<
https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D11-2B-2D-2BAuthorization-2BInterface=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=NQw-765HFBIbCYIZLpYIJrvCaZ4-Ml0L1e4z4wkx8fg=
>) with operation = DELETE and resource = TOPIC.

You know that when a request comes in there is the "authorizer" first
which is based on KIP-11 and then the operation is called and then you
could have policy.

I.e. for create topics, first the "authorizer" is called in the KafkaApis
(operation = CREATE, resource = TOPIC) and then the CreateTopicPolicy is
applied in the AdminManager.


In any case what KIP-170 is proposing is not good. Maybe we could have
operation = DELETE and a new resource = MESSAGE/RECORD.

What could be useful use cases for having a RecordsDeletePolicy ? Records
can't be deleted for a topic name ? Starting from a specific offset ?


Paolo Patierno
Senior Software Engineer (IoT) @ Red Hat
Microsoft MVP on Azure & IoT
Microsoft Azure Advisor

Twitter : @ppatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter.com_ppatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=XDNIqoF_Lv9mDGsBvFaRn8bazYxBs3a4lWKrekvRxrw=
>
Linkedin : paolopatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__it.linkedin.com_in_paolopatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=6dww9uXw2L0IZ3JKqvoxImEA07L5d7ORCtWi3_wxwcw=
>
Blog : DevExperience<
https://urldefense.proofpoint.com/v2/url?u=http-3A__paolopatierno.wordpress.com_=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=RcYQ4n1Fcl2vHgy2Aj_J9wUO3ZUaFgl8vCZ4slCD5ro=
>



From: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 19, 2017 8:55 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the
new Admin Client API

Hi Paolo,

Thanks for the KIP.

What errors can be anticipated for the API method (See
https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.apache.org_jira_browse_KAFKA-2D5445-29-3F=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=EaM1W6ZCxDBgKxKpVthbGgK2riVb9EESHu3EfeLMvOQ=


It's not mentioned in KIP-107, but maybe now is a good time to consider
whether there should be some kind of DeleteRecordsPolicy, like there is
for
creating topics? Note: KIP-170 proposes a TopicDeletePolicy, but I don't
think you could use that as currently proposed, because it would be unable
to distinguish the deletion of an entire topic f

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Edoardo Comar]

Hi Paolo,

our the motivation for suggesting a delete policy as described in KIP-170 
was to avoid deletion of topics that have a specific usage (essential to 
other services).
As you say, it would be entirely replaceable by an Authorizer 
implementation that performs the equivalent check.

thanks
Edo
--

Edoardo Comar

IBM Message Hub

IBM UK Ltd, Hursley Park, SO21 2JN



From:   Paolo Patierno <ppatie...@live.com>
To: "dev@kafka.apache.org" <dev@kafka.apache.org>
Date:   26/09/2017 14:10
Subject:        Re: [DISCUSS] KIP-204 : adding records deletion operation 
to the new Admin Client API



Hi Tom,


the KIP-170 doesn't propose to use a TopicDeletePolicy as policy classes 
are meant to be.

It's referring to the authorization interface (KIP-11<
https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D11-2B-2D-2BAuthorization-2BInterface=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=NQw-765HFBIbCYIZLpYIJrvCaZ4-Ml0L1e4z4wkx8fg=
 
>) with operation = DELETE and resource = TOPIC.

You know that when a request comes in there is the "authorizer" first 
which is based on KIP-11 and then the operation is called and then you 
could have policy.

I.e. for create topics, first the "authorizer" is called in the KafkaApis 
(operation = CREATE, resource = TOPIC) and then the CreateTopicPolicy is 
applied in the AdminManager.


In any case what KIP-170 is proposing is not good. Maybe we could have 
operation = DELETE and a new resource = MESSAGE/RECORD.

What could be useful use cases for having a RecordsDeletePolicy ? Records 
can't be deleted for a topic name ? Starting from a specific offset ?


Paolo Patierno
Senior Software Engineer (IoT) @ Red Hat
Microsoft MVP on Azure & IoT
Microsoft Azure Advisor

Twitter : @ppatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__twitter.com_ppatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=XDNIqoF_Lv9mDGsBvFaRn8bazYxBs3a4lWKrekvRxrw=
 
>
Linkedin : paolopatierno<
https://urldefense.proofpoint.com/v2/url?u=http-3A__it.linkedin.com_in_paolopatierno=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=6dww9uXw2L0IZ3JKqvoxImEA07L5d7ORCtWi3_wxwcw=
 
>
Blog : DevExperience<
https://urldefense.proofpoint.com/v2/url?u=http-3A__paolopatierno.wordpress.com_=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=RcYQ4n1Fcl2vHgy2Aj_J9wUO3ZUaFgl8vCZ4slCD5ro=
 
>



From: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 19, 2017 8:55 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the 
new Admin Client API

Hi Paolo,

Thanks for the KIP.

What errors can be anticipated for the API method (See
https://urldefense.proofpoint.com/v2/url?u=https-3A__issues.apache.org_jira_browse_KAFKA-2D5445-29-3F=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=EaM1W6ZCxDBgKxKpVthbGgK2riVb9EESHu3EfeLMvOQ=
 


It's not mentioned in KIP-107, but maybe now is a good time to consider
whether there should be some kind of DeleteRecordsPolicy, like there is 
for
creating topics? Note: KIP-170 proposes a TopicDeletePolicy, but I don't
think you could use that as currently proposed, because it would be unable
to distinguish the deletion of an entire topic from the deletion of some
records within a topic.

Thanks again,

Tom


On 18 September 2017 at 21:06, Dong Lin <lindon...@gmail.com> wrote:

> +1 (non-binding)
>
> On Mon, Sep 18, 2017 at 1:04 PM, Ted Yu <yuzhih...@gmail.com> wrote:
>
> > +1
> >
> > On Mon, Sep 18, 2017 at 9:19 AM, Paolo Patierno <ppatie...@live.com>
> > wrote:
> >
> > > Hi devs,
> > >
> > >
> > > I'd like to start a discussion around adding the delete records
> > operation,
> > > already available at protocol level and in the "legacy" Admin Client 
in
> > > Scala, to the "new" Admin Client API in Java.
> > >
> > >
> > > 
https://urldefense.proofpoint.com/v2/url?u=https-3A__cwiki.apache.org_confluence_display_KAFKA_KIP-2D=DwIFAw=jf_iaSHvJObTbx-siA1ZOg=EzRhmSah4IHsUZVekRUIINhltZK7U0OaeRo7hgW4_tQ=7f9IdD9OiWfZojnX4FN8OUimAl_0Q72yxvE8Nz_QdpU=_ojBO1P6pVw7qcLfB7T9wAgFWqhfd-JswSnJ8hRtoEs=
 

> > > 
204+%3A+adding+records+deletion+operation+to+the+new+Admin+Client+API
> > >
> > >
> > > Thanks,
> > >
> > >
> > > Paolo Patierno
> > > Senior Software 

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi Tom,


the KIP-170 doesn't propose to use a TopicDeletePolicy as policy classes are 
meant to be.

It's referring to the authorization interface 
(KIP-11<https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface>)
 with operation = DELETE and resource = TOPIC.

You know that when a request comes in there is the "authorizer" first which is 
based on KIP-11 and then the operation is called and then you could have policy.

I.e. for create topics, first the "authorizer" is called in the KafkaApis 
(operation = CREATE, resource = TOPIC) and then the CreateTopicPolicy is 
applied in the AdminManager.


In any case what KIP-170 is proposing is not good. Maybe we could have 
operation = DELETE and a new resource = MESSAGE/RECORD.

What could be useful use cases for having a RecordsDeletePolicy ? Records can't 
be deleted for a topic name ? Starting from a specific offset ?


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: Tom Bentley <t.j.bent...@gmail.com>
Sent: Tuesday, September 19, 2017 8:55 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP-204 : adding records deletion operation to the new 
Admin Client API

Hi Paolo,

Thanks for the KIP.

What errors can be anticipated for the API method (See
https://issues.apache.org/jira/browse/KAFKA-5445)?

It's not mentioned in KIP-107, but maybe now is a good time to consider
whether there should be some kind of DeleteRecordsPolicy, like there is for
creating topics? Note: KIP-170 proposes a TopicDeletePolicy, but I don't
think you could use that as currently proposed, because it would be unable
to distinguish the deletion of an entire topic from the deletion of some
records within a topic.

Thanks again,

Tom


On 18 September 2017 at 21:06, Dong Lin <lindon...@gmail.com> wrote:

> +1 (non-binding)
>
> On Mon, Sep 18, 2017 at 1:04 PM, Ted Yu <yuzhih...@gmail.com> wrote:
>
> > +1
> >
> > On Mon, Sep 18, 2017 at 9:19 AM, Paolo Patierno <ppatie...@live.com>
> > wrote:
> >
> > > Hi devs,
> > >
> > >
> > > I'd like to start a discussion around adding the delete records
> > operation,
> > > already available at protocol level and in the "legacy" Admin Client in
> > > Scala, to the "new" Admin Client API in Java.
> > >
> > >
> > > https://cwiki.apache.org/confluence/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/>
> > >
> >
>

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Tom Bentley]

Hi Paolo,

Thanks for the KIP.

What errors can be anticipated for the API method (See
https://issues.apache.org/jira/browse/KAFKA-5445)?

It's not mentioned in KIP-107, but maybe now is a good time to consider
whether there should be some kind of DeleteRecordsPolicy, like there is for
creating topics? Note: KIP-170 proposes a TopicDeletePolicy, but I don't
think you could use that as currently proposed, because it would be unable
to distinguish the deletion of an entire topic from the deletion of some
records within a topic.

Thanks again,

Tom


On 18 September 2017 at 21:06, Dong Lin  wrote:

> +1 (non-binding)
>
> On Mon, Sep 18, 2017 at 1:04 PM, Ted Yu  wrote:
>
> > +1
> >
> > On Mon, Sep 18, 2017 at 9:19 AM, Paolo Patierno 
> > wrote:
> >
> > > Hi devs,
> > >
> > >
> > > I'd like to start a discussion around adding the delete records
> > operation,
> > > already available at protocol level and in the "legacy" Admin Client in
> > > Scala, to the "new" Admin Client API in Java.
> > >
> > >
> > > https://cwiki.apache.org/confluence/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
> > > Linkedin : paolopatierno
> > > Blog : DevExperience
> > >
> >
>

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Dong Lin]

+1 (non-binding)

On Mon, Sep 18, 2017 at 1:04 PM, Ted Yu  wrote:

> +1
>
> On Mon, Sep 18, 2017 at 9:19 AM, Paolo Patierno 
> wrote:
>
> > Hi devs,
> >
> >
> > I'd like to start a discussion around adding the delete records
> operation,
> > already available at protocol level and in the "legacy" Admin Client in
> > Scala, to the "new" Admin Client API in Java.
> >
> >
> > https://cwiki.apache.org/confluence/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
> > Linkedin : paolopatierno
> > Blog : DevExperience
> >
>

Re: [DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Ted Yu]

+1

On Mon, Sep 18, 2017 at 9:19 AM, Paolo Patierno  wrote:

> Hi devs,
>
>
> I'd like to start a discussion around adding the delete records operation,
> already available at protocol level and in the "legacy" Admin Client in
> Scala, to the "new" Admin Client API in Java.
>
>
> https://cwiki.apache.org/confluence/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
> Linkedin : paolopatierno
> Blog : DevExperience
>

[DISCUSS] KIP-204 : adding records deletion operation to the new Admin Client API

[Paolo Patierno]

Hi devs,


I'd like to start a discussion around adding the delete records operation, 
already available at protocol level and in the "legacy" Admin Client in Scala, 
to the "new" Admin Client API in Java.


https://cwiki.apache.org/confluence/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
Linkedin : paolopatierno
Blog : DevExperience