Jun, KIP-50 is now updated. Mind taking a look.
On Mon, Apr 4, 2016 at 10:40 PM, Ashish Singh <asi...@cloudera.com> wrote: > Jun, > > Your suggested approach works, will update the KIP and re-initiate voting. > Thanks! > > On Sun, Apr 3, 2016 at 8:37 AM, Jun Rao <j...@confluent.io> wrote: > >> Ashish, >> >> For the two benefits that you listed, fail fast can be done just in the >> implementation w/o getSupportedPrincipalTypes(). For avoiding the same >> check in all implementations, I think Grant is thinking of adding some ACL >> RPC request/response btw the client and broker directly, instead of >> writing >> the ACL to ZK directly in the future. If we do that, the right place to do >> any sanity check is probably in each implementation instead of in >> AclCommand since AclCommand won't necessarily be the only entry point for >> changing ACLs. So, I am still not quite convinced that we should >> getSupportedPrincipalTypes() >> right now. It's easy to add a new method to the interface, but hard to >> remove/change an existing method. Grant, could you comment on the latter? >> >> Thanks, >> >> Jun >> >> On Sat, Apr 2, 2016 at 11:43 AM, Ashish Singh <asi...@cloudera.com> >> wrote: >> >> > Hello Jun, >> > >> > >> > On Fri, Apr 1, 2016 at 9:57 PM, Jun Rao <j...@confluent.io> wrote: >> > >> > > Ashish, >> > > >> > > Thanks for the KIP. >> > > >> > > It seems that a specific implementation of Authorizer can reject >> invalid >> > > user type in addAcl/removeAcl without needing the new >> > > getSupportedPrincipalTypes() >> > > method, right? It's probably useful to provide the supported user >> types >> > as >> > > information through CLI (e.g., when --help is specified). Then, there >> may >> > > other information that a specific authorizer may want to provide. So, >> if >> > > this is just informational, would it be better to add sth like >> > > getDescription() in the Authorizer interface and expose that through >> CLI? >> > > >> > Providing information is definitely an important reason, some other >> reasons >> > were to fail fast and to avoid same check in all implementations. I >> agree >> > having a generic getDescription() will be handy for authorizer >> > implementations to provide more implementation specific info, including >> > supported principal types and more. However, do you think other two >> reasons >> > I mentioned can convince you for current proposal? >> > >> > > >> > > Jun >> > > >> > > On Wed, Mar 30, 2016 at 11:02 AM, Ashish Singh <asi...@cloudera.com> >> > > wrote: >> > > >> > > > Hi Guys, >> > > > >> > > > I would like to open the vote for KIP-50. >> > > > >> > > > KIP: >> > > > >> > > > >> > > >> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types >> > > > >> > > > Discuss thread: here >> > > > < >> > > > >> > > >> > >> http://mail-archives.apache.org/mod_mbox/kafka-dev/201603.mbox/%3CCAGQG9cUCLDO0owdziDcL9iStXNF1wURyVNcEZedQJg%3DUuC7j%3DQ%40mail.gmail.com%3E >> > > > > >> > > > >> > > > JIRA: https://issues.apache.org/jira/browse/KAFKA-3186 >> > > > >> > > > PR: https://github.com/apache/kafka/pull/861 >> > > > >> > > > -- >> > > > >> > > > Regards, >> > > > Ashish >> > > > >> > > >> > >> > >> > >> > -- >> > >> > Regards, >> > Ashish >> > >> > > > > -- > > Regards, > Ashish > -- Regards, Ashish