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