Edoardo, thanks for the KIP. I think it's a good idea overall. +1 especially for including Session/Principal in the API. (#2 mentioned by Ismael)
Also, the AlterPolicy should get the same info as create + delete (#4). Cheers, Roger On Thu, Sep 7, 2017 at 8:43 AM, Ismael Juma <ism...@juma.me.uk> wrote: > Hi Tom, > > Maybe we can discuss that as part of KIP-179. I don't think we should > introduce a separate interface for the changes suggested here. > > Ismael > > On Tue, Sep 5, 2017 at 7:05 PM, Tom Bentley <t.j.bent...@gmail.com> wrote: > > > Hi Edoardo, > > > > KIP-179 will support changing topic configurations via the AdminClient > and > > this would open a loophole to avoid the current CreateTopicPolicy: > > > > 1. Create a topic that sticks to the policy > > 2. Modify the topic after creation to something which would have violated > > the policy. > > > > For this reason KIP-179 proposes to apply the create topic policy to > topic > > modification too. If there were going to be a new "V2" interface (as > > opposed to changing the RequestMetadata) I think it would be beneficial > if > > the name reflected that it applied to updates too. > > CreateOrAlterTopicPolicy? Though obviously that name doesn't reflect that > > its heritage in the original CreateTopicPolicy. > > > > Cheers, > > > > Tom > > > > On 5 September 2017 at 18:48, Edoardo Comar <edoco...@gmail.com> wrote: > > > > > Hi Ismael, > > > I was on leave for a long while. I will update the KIP. > > > > > > Edo > > > > > > On 5 September 2017 at 11:42, Ismael Juma <ism...@juma.me.uk> wrote: > > > > > > > Hi Edoardo, > > > > > > > > Do you intend to update the KIP to avoid the introduction of another > > > > interface? > > > > > > > > Ismael > > > > > > > > On Fri, Jun 23, 2017 at 5:47 PM, Edoardo Comar <eco...@uk.ibm.com> > > > wrote: > > > > > > > > > Thanks for the thoughts Ismael > > > > > > > > > > > 1. Have you considered extending RequestMetadata with the > > additional > > > > > > information you need? We could add Cluster to it, which has topic > > > > > > assignment information, for example. This way, there would be no > > need > > > > > for a > > > > > > V2 interface. > > > > > > > > > > 1. I hadn't thought of it - but this seems a feasible alternative. > > > > > > > > > > The XXXPolicy.RequestMetadata could be enriched to include more > > > > > information - > > > > > for backward compatibility with existing Policies we would only add > > > > > methods to these classes. > > > > > > > > > > Presumably the number of Admin Request is not huge so we should not > > be > > > > too > > > > > worried > > > > > about the overhead of populating a org.apache.kafka.common.Cluster > > for > > > > > every admin request , right ? > > > > > > > > > > The only feature I can't see how to easily support with the > enriched > > > > > XXXPolicy.RequestMetadata is how to check > > > > > for a topic being marked for deletion using the Cluster information > > > > > > > > > > This check is useful for us in excluding such outliers when > counting > > > the > > > > > number of partitions. > > > > > > > > > > > > > > > > 2. Something else that could be useful is passing an instance of > > > > > `Session` > > > > > > so that one can provide custom behaviour depending on the logged > in > > > > > user. > > > > > > Would this be useful? > > > > > 2. Definitely I would expect that a general solution includes the > > > Session > > > > > or the KafkaPrincipal associated with it > > > > > (the latter may be a simpler dependency for the > > > XXXPolicy.RequestMetadata > > > > > > > > > > > > > > > > 3. For the delete case, we may consider passing a class instead > of > > > just > > > > > a > > > > > > string to the validate method so that we have options if we need > to > > > > > extend > > > > > > it. > > > > > > > > > > 3. Agree, we should have the DeletePolicy define its > RequestMetadata > > > > > class, too > > > > > > > > > > > > > > > > 4. Do we want to enhance the AlterConfigs policy as well? > > > > > > > > > > 4. I don't see why not :-) > > > > > > > > > > > > > > > > > > > > thanks > > > > > Edo > > > > > -------------------------------------------------- > > > > > > > > > > Edoardo Comar > > > > > > > > > > IBM Message Hub > > > > > > > > > > > > > > > isma...@gmail.com wrote on 22/06/2017 15:05:06: > > > > > > > > > > > From: Ismael Juma <ism...@juma.me.uk> > > > > > > To: dev@kafka.apache.org > > > > > > Date: 22/06/2017 15:05 > > > > > > Subject: Re: [DISCUSS] KIP-170: Enhanced TopicCreatePolicy and > > > > > > introduction of TopicDeletePolicy > > > > > > Sent by: isma...@gmail.com > > > > > > > > > > > > Thanks for the KIP, Edoardo. A few comments: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > On Thu, Jun 22, 2017 at 2:41 PM, Edoardo Comar < > eco...@uk.ibm.com> > > > > > wrote: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > We've drafted "KIP-170: Enhanced TopicCreatePolicy and > > introduction > > > > of > > > > > > > TopicDeletePolicy" for discussion: > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > 170%3A+Enhanced+ > > > > > > > TopicCreatePolicy+and+introduction+of+TopicDeletePolicy > > > > > > > > > > > > > > Please take a look. Your feedback is welcome and much needed. > > > > > > > > > > > > > > Thanks, > > > > > > > Edoardo > > > > > > > -------------------------------------------------- > > > > > > > Edoardo Comar > > > > > > > IBM Message Hub > > > > > > > eco...@uk.ibm.com > > > > > > > IBM UK Ltd, Hursley Park, SO21 2JN > > > > > > > Unless stated otherwise above: > > > > > > > IBM United Kingdom Limited - Registered in England and Wales > with > > > > > number > > > > > > > 741598. > > > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, > > Hampshire > > > > PO6 > > > > > 3AU > > > > > > > > > > > > > > > > > Unless stated otherwise above: > > > > > IBM United Kingdom Limited - Registered in England and Wales with > > > number > > > > > 741598. > > > > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire > > PO6 > > > > 3AU > > > > > > > > > > > > > > > > > > > > > -- > > > "When the people fear their government, there is tyranny; when the > > > government fears the people, there is liberty." [Thomas Jefferson] > > > > > >