Hey Ismael, Thanks for the KIP. This is definitely useful.
Does the KIP apply the topic creation policy to ZK-based topic creation? If not, which seems to be the case from my understanding, should we have a new broker config to disable ZK-based topic creation? This seems necessary to prevent user from using stray builds to evade the topic creation policy. Thanks, Dong On Mon, Jan 9, 2017 at 1:42 PM, Roger Hoover <roger.hoo...@gmail.com> wrote: > Got it. Thanks, Ismael. > > On Mon, Jan 9, 2017 at 10:42 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > Hi Roger, > > > > That's a good question. The server defaults are passed via the > `configure` > > method of the `Configurable` interface that is implemented by > > `CreateTopicPolicy`. I'll mention this explicitly in the KIP. > > > > Ismael > > > > On Mon, Jan 9, 2017 at 6:04 PM, Roger Hoover <roger.hoo...@gmail.com> > > wrote: > > > > > This is great. Thanks, Ismael. > > > > > > One question. When TopicDetails are passed to the policy > implementation, > > > would the server defaults already have been merged? If not, I think > the > > > policy also needs access to the server defaults. > > > > > > Cheers, > > > > > > Roger > > > > > > On Fri, Jan 6, 2017 at 9:26 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > > > > > Thanks for the review Jun. Yes, that's a good point, I have updated > the > > > > KIP. > > > > > > > > Ismael > > > > > > > > On Fri, Jan 6, 2017 at 5:15 PM, Jun Rao <j...@confluent.io> wrote: > > > > > > > > > Hi, Ismael, > > > > > > > > > > Thanks for the KIP. Looks reasonable to me. To be consistent with > the > > > > > pattern used in other pluggable interfaces, we probably should make > > the > > > > new > > > > > interface configurable and closable? > > > > > > > > > > Jun > > > > > > > > > > On Fri, Jan 6, 2017 at 4:16 AM, Ismael Juma <ism...@juma.me.uk> > > wrote: > > > > > > > > > > > Thanks Dan and Colin for the feedback. I updated the KIP to > include > > > the > > > > > > addition of a validation mode. Since we need to bump the protocol > > > > version > > > > > > for that, I also added an error message per topic to the > response. > > I > > > > had > > > > > > the latter as "Future Work", but I actually felt that it should > be > > in > > > > the > > > > > > first version (good to have feedback confirming that). > > > > > > > > > > > > Let me know if the changes look good to you. > > > > > > > > > > > > Ismael > > > > > > > > > > > > On Thu, Jan 5, 2017 at 9:54 PM, Colin McCabe <cmcc...@apache.org > > > > > > wrote: > > > > > > > > > > > > > Yeah, I agree... having a validation mode would be nice. We > > should > > > > be > > > > > > > explicit that passing validation doesn't 100% guarantee that a > > > > > > > subsequent call to create the topic will succeed, though. > There > > is > > > > an > > > > > > > obvious race condition there-- for example, with a plugin which > > > > > consults > > > > > > > some external authentication system, there could be a change to > > the > > > > > > > privileges in between validation and attempted creation. > > > > > > > > > > > > > > It also seems like we should try to provide a helpful exception > > > > message > > > > > > > for the cases where topic creation fails. This might involve > > > adding > > > > > > > more detail about error conditions to CreateTopicsRequest... > > right > > > > now > > > > > > > it just returns an error code, but a text message would be a > nice > > > > > > > addition. > > > > > > > > > > > > > > cheers, > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > On Thu, Jan 5, 2017, at 13:41, dan wrote: > > > > > > > > it would be nice to have a dry-run or validate ability added > to > > > > this > > > > > > kip. > > > > > > > > since we are offloading validation to a 3rd party > implementor a > > > > > random > > > > > > > > user > > > > > > > > can't know a priori (based solely on kafka configs) whether a > > > call > > > > > > should > > > > > > > > succeed without actually creating the topic. > > > > > > > > > > > > > > > > a similar case is in connect where there is a separate > endpoint > > > > > > > > <https://github.com/apache/kafka/blob/trunk/connect/ > > > > > > > runtime/src/main/java/org/apache/kafka/connect/runtime/rest/ > > > > resources/ > > > > > > > ConnectorPluginsResource.java#L49-L58> > > > > > > > > to attempt to validate a connect configuration without > actually > > > > > > creating > > > > > > > > the connector. > > > > > > > > > > > > > > > > thanks > > > > > > > > dan > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Jan 5, 2017 at 7:34 AM, Ismael Juma < > ism...@juma.me.uk > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > We've posted "KIP-108: Create Topic Policy" for discussion: > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > > > > 108%3A+Create+Topic+Policy > > > > > > > > > > > > > > > > > > Please take a look. Your feedback is appreciated. > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Ismael > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
