Yes, if we have Java interface, then we won’t need Scala interface. Thanks
Bosco On 4/8/16, 9:04 AM, "Ashish Singh" <asi...@cloudera.com> wrote: >Thanks for the input Don. One of the possible paths for Option 2 is to >completely drop Scala interface, would that be Ok with you folks? > >On Thursday, April 7, 2016, Don Bosco Durai <bo...@apache.org> wrote: > >> Ranger team would prefer option #2. Right now, we have to access some of >> the nested constants using constructs like Group$.MODULE$, which is not >> intuitive in Java. >> >> Thanks >> >> Bosco >> >> >> >> >> On 4/7/16, 4:30 PM, "Ashish Singh" <asi...@cloudera.com <javascript:;>> >> wrote: >> >> >Harsha/ Don, >> > >> >Are you guys OK with option 2? I am not aware of all the existing >> >authorizer implementations, however ranger has one for sure. Getting >> direct >> >feedback from you guys will be really valuable. >> > >> >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma <ism...@juma.me.uk >> <javascript:;>> wrote: >> > >> >> Hi Don, >> >> >> >> This is true in Java 7, but Java 8 introduces default methods and this >> >> workaround is no longer required. During the Interceptor KIP >> discussion, it >> >> was decided that it was fine to stick to interfaces given that we are >> >> likely to move to Java 8 in the nearish future (probably no later than >> the >> >> Java 9 release). >> >> >> >> Ismael >> >> >> >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai <bo...@apache.org >> <javascript:;>> wrote: >> >> >> >> > Hi Ashish >> >> > >> >> > If we are going by option #2, then I can suggest we give an abstract >> >> > implementation of the Interface and recommend anyone implementing >> their >> >> own >> >> > plugin to extend from the abstract class, rather than implement the >> >> > interface? >> >> > >> >> > The advantage is, in the future if we add add any new methods in the >> >> > Interface (e.g. Similar to getDescription()), then we can give a dummy >> >> > implementation of the new method and this won’t break the compilation >> of >> >> > any external implementation. Else over the time it will be challenging >> >> for >> >> > anyone customizing the implementation to keep track of changes to the >> >> > Interface. >> >> > >> >> > Thanks >> >> > >> >> > Bosco >> >> > >> >> > >> >> > >> >> > >> >> > On 4/7/16, 11:21 AM, "Ashish Singh" <asi...@cloudera.com >> <javascript:;>> wrote: >> >> > >> >> > >Hello Harsha, >> >> > > >> >> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha <m...@harsha.io >> <javascript:;>> wrote: >> >> > > >> >> > >"My only ask is to have this in 0.10. As Jay pointed out, right now >> >> > >> there >> >> > >> are not many implementations out there, we might want to fix it >> ASAP." >> >> > >> >> >> > >> Probably there aren't many implementations but there are lot of >> users >> >> > >> using these implementations in production clusters. >> >> > >> Isn't this going to break the rolling upgrade? >> >> > > >> >> > >It will and it is a concern, in my previous mail I have mentioned >> this >> >> as >> >> > >an issue if we choose to go this route. However, if we actually >> decide >> >> to >> >> > >do this, I would say it is better to do it sooner than later, as >> fewer >> >> > >implementations will be affected. Below is excerpt from my previous >> >> mail. >> >> > > >> >> > >Increase scope of KIP-50 to move authorizer and related classes to a >> >> > >separate package. The new package will have java interface. This will >> >> > allow >> >> > >implementations to not depend on kafka core and just on authorizer >> >> > package, >> >> > >make authorization interface follow kafka’s coding standards and will >> >> > allow >> >> > >java implementations to be cleaner. We can either completely drop >> scala >> >> > >interface, which might be a pain for existing implementations, or we >> can >> >> > >have scala interface wrap java interface. Later allows a cleaner >> >> > >deprecation path for existing scala authorizer interface, however it >> may >> >> > or >> >> > >may not be feasible as Kafka server will have to somehow decide which >> >> > >interface it should be looking for while loading authorizer >> >> > implementation, >> >> > >this can probably be solved with a config or some reflection. If we >> >> choose >> >> > >to go this route, I can dig deeper. >> >> > > >> >> > >If we go with option 2 and commit on getting this in ASAP, >> preferably in >> >> > >0.10, there will be fewer implementations that will be affected. >> >> > > >> >> > >and also moving to Java , >> >> > >> a authorizer implementation going to run inside a KafkaBroker and I >> >> > >> don't see why this is necessary to move to clients package. >> >> > >> Are we planning on introducing common module to have it >> independent of >> >> > >> broker and client code? >> >> > >> >> >> > >Yes, I think that would take away the requirement of depending on >> Kafka >> >> > >core from authorizer implementations. Do you suggest otherwise? >> >> > > >> >> > > >> >> > >> -Harsha >> >> > >> >> >> > >> On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote: >> >> > >> > We might want to take a call here. Following are the options. >> >> > >> > >> >> > >> > 1. Let KIP-50 be the way it is, i.e., just add >> getDescription() >> >> to >> >> > >> > existing scala authorizer interface. It will break binary >> >> > >> > compatibility >> >> > >> > (only when using CLI and/or AdminCommand from >= 0.10 against >> >> > >> > authorizer >> >> > >> > implementations based on 0.9.). If we go this route, it is a >> >> > simpler >> >> > >> > change >> >> > >> > and existing implementations won’t have to change anything on >> >> their >> >> > >> > end. >> >> > >> > 2. Increase scope of KIP-50 to move authorizer and related >> >> classes >> >> > to >> >> > >> > a >> >> > >> > separate package. The new package will have java interface. >> This >> >> > will >> >> > >> > allow >> >> > >> > implementations to not depend on kafka core and just on >> >> authorizer >> >> > >> > package, >> >> > >> > make authorization interface follow kafka’s coding standards >> and >> >> > will >> >> > >> > allow >> >> > >> > java implementations to be cleaner. We can either completely >> drop >> >> > >> > scala >> >> > >> > interface, which might be a pain for existing >> implementations, or >> >> > we >> >> > >> > can >> >> > >> > have scala interface wrap java interface. Later allows a >> cleaner >> >> > >> > deprecation path for existing scala authorizer interface, >> however >> >> > it >> >> > >> > may or >> >> > >> > may not be feasible as Kafka server will have to somehow >> decide >> >> > which >> >> > >> > interface it should be looking for while loading authorizer >> >> > >> > implementation, >> >> > >> > this can probably be solved with a config or some reflection. >> If >> >> we >> >> > >> > choose >> >> > >> > to go this route, I can dig deeper. >> >> > >> > >> >> > >> > If we decide to go with option 1, I think it would be fair to say >> >> that >> >> > >> > scala authorizer interface will be around for some time, as there >> >> > will be >> >> > >> > more implementations relying on it. If we go with option 2 and >> >> commit >> >> > on >> >> > >> > getting this in ASAP, preferably in 0.10, there will be fewer >> >> > >> > implementations that will be affected. >> >> > >> > >> >> > >> > *Another thing to notice is that scala authorizer interface is >> not >> >> > >> > annotated as unstable.* >> >> > >> > >> >> > >> > >> >> > >> > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh < >> asi...@cloudera.com <javascript:;>> >> >> > >> wrote: >> >> > >> > >> >> > >> > > I see value in minimizing breaking changes and I do not oppose >> the >> >> > >> idea of >> >> > >> > > increasing scope of KIP-50 to move auth interface to java. >> >> > >> > > >> >> > >> > > As authorizer implementations do not really need to depend on >> >> Kafka >> >> > >> core, >> >> > >> > > I would suggest that we keep authorizer interface and its >> >> components >> >> > >> in a >> >> > >> > > separate package. I share the concern that right now using >> >> Resource, >> >> > >> > > Operation, etc, in java implementations is messy. I had to deal >> >> with >> >> > >> lot of >> >> > >> > > it while writing Apache Sentry plugin. >> >> > >> > > >> >> > >> > > My only ask is to have this in 0.10. As Jay pointed out, right >> now >> >> > >> there >> >> > >> > > are not many implementations out there, we might want to fix it >> >> > ASAP. >> >> > >> I can >> >> > >> > > only speak of Sentry integration and I think 0.10 will be the >> best >> >> > for >> >> > >> such >> >> > >> > > a change, as I should be able to adopt the changes in Sentry >> >> > >> integration >> >> > >> > > before a lot of users start using it. >> >> > >> > > >> >> > >> > > On Wed, Apr 6, 2016 at 9:25 AM, Ismael Juma <ism...@juma.me.uk >> <javascript:;>> >> >> > wrote: >> >> > >> > > >> >> > >> > >> It is small, but breaks binary compatibility. >> >> > >> > >> >> >> > >> > >> Ismael >> >> > >> > >> >> >> > >> > >> On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke < >> ghe...@cloudera.com <javascript:;> >> >> > >> >> > >> wrote: >> >> > >> > >> >> >> > >> > >> > KIP-50 as defined is very small. I don't see any harm in >> >> putting >> >> > it >> >> > >> in >> >> > >> > >> as >> >> > >> > >> > is and then tackling the follow up work. >> >> > >> > >> > >> >> > >> > >> > >> >> > >> > >> > On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma < >> >> ism...@juma.me.uk <javascript:;>> >> >> > >> wrote: >> >> > >> > >> > >> >> > >> > >> > > Thanks Grant. I wonder if KIP-50 should just be done as >> part >> >> of >> >> > >> this >> >> > >> > >> > work. >> >> > >> > >> > > >> >> > >> > >> > > Ismael >> >> > >> > >> > > >> >> > >> > >> > > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke < >> >> > ghe...@cloudera.com <javascript:;>> >> >> > >> > >> wrote: >> >> > >> > >> > > >> >> > >> > >> > > > My work with KIP-4 found that many of the Scala classes >> >> used >> >> > in >> >> > >> the >> >> > >> > >> > > > Authorizer interface are needed in the Clients package >> when >> >> > >> adding >> >> > >> > >> the >> >> > >> > >> > > > various ACL requests and responses. I also found that we >> >> > don't >> >> > >> have >> >> > >> > >> > > > standard Exceptions defined for the authorizer >> interface. >> >> > This >> >> > >> means >> >> > >> > >> > that >> >> > >> > >> > > > when I add the Authorizer calls to the broker and wire >> >> > >> protocols all >> >> > >> > >> > > > exceptions will be reported as an "Unknown Error" back >> to >> >> the >> >> > >> user >> >> > >> > >> via >> >> > >> > >> > > the >> >> > >> > >> > > > wire protocol. >> >> > >> > >> > > > >> >> > >> > >> > > > I have written more about it on the KIP-4 wiki and >> created >> >> > >> jiras to >> >> > >> > >> > track >> >> > >> > >> > > > those issues (See below). I think we should wrap up this >> >> KIP >> >> > as >> >> > >> is >> >> > >> > >> and >> >> > >> > >> > > > tackle the Java/Exception changes as a part of those >> >> > jiras/kips. >> >> > >> > >> > > > >> >> > >> > >> > > > - KIP-4 "Follow Up Changes" >> >> > >> > >> > > > < >> >> > >> > >> > > > >> >> > >> > >> > > >> >> > >> > >> > >> >> > >> > >> >> >> > >> >> >> > >> >> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes >> >> > >> > >> > > > > >> >> > >> > >> > > > - KAFKA-3509 < >> >> > >> https://issues.apache.org/jira/browse/KAFKA-3509>: >> >> > >> > >> > > > Provide >> >> > >> > >> > > > an Authorizer interface using the Java client >> enumerator >> >> > >> classes >> >> > >> > >> > > > - KAFKA-3507 < >> >> > >> https://issues.apache.org/jira/browse/KAFKA-3507>: >> >> > >> > >> > > Define >> >> > >> > >> > > > standard exceptions for the Authorizer interface >> >> > >> > >> > > > >> >> > >> > >> > > > Thank you, >> >> > >> > >> > > > Grant >> >> > >> > >> > > > >> >> > >> > >> > > > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps < >> >> j...@confluent.io <javascript:;> >> >> > > >> >> > >> > >> wrote: >> >> > >> > >> > > > >> >> > >> > >> > > > > Hey Ismael, >> >> > >> > >> > > > > >> >> > >> > >> > > > > Yeah I think this is a minor cleanliness thing. Since >> >> this >> >> > is >> >> > >> kind >> >> > >> > >> > of a >> >> > >> > >> > > > > power user interface I don't feel strongly either way. >> >> > >> > >> > > > > >> >> > >> > >> > > > > My motivation with Scala is just that we've tried to >> move >> >> > to >> >> > >> > >> having >> >> > >> > >> > the >> >> > >> > >> > > > > public interfaces be Java, and as a group we >> definitely >> >> > >> struggled >> >> > >> > >> a >> >> > >> > >> > lot >> >> > >> > >> > > > > with understanding and maintaining Scala >> compatibility in >> >> > the >> >> > >> > >> older >> >> > >> > >> > > > > clients. >> >> > >> > >> > > > > >> >> > >> > >> > > > > -Jay >> >> > >> > >> > > > > >> >> > >> > >> > > > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma < >> >> > >> ism...@juma.me.uk <javascript:;>> >> >> > >> > >> > > wrote: >> >> > >> > >> > > > > >> >> > >> > >> > > > > > Hi Jay, >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps < >> >> > j...@confluent.io <javascript:;> >> >> > >> > >> >> > >> > >> > wrote: >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > > Given that we're breaking compatibility anyway >> should >> >> > we: >> >> > >> > >> > > > > > > >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > We are not breaking source compatibility since the >> new >> >> > >> method >> >> > >> > >> has a >> >> > >> > >> > > > > default >> >> > >> > >> > > > > > implementation. I take it that you mean binary >> >> > >> compatibility? >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > > 1. Remove the get prefix on this method and the >> >> > existing >> >> > >> one >> >> > >> > >> > which >> >> > >> > >> > > > > > violate >> >> > >> > >> > > > > > > our own code style guidelines (Oops! Kind of sad >> we >> >> > went >> >> > >> > >> through >> >> > >> > >> > > the >> >> > >> > >> > > > > > whole >> >> > >> > >> > > > > > > KIP process and no one even flagged this) >> >> > >> > >> > > > > > > >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > I did flag this during the discussion and Ashish >> said >> >> he >> >> > >> would >> >> > >> > >> > change >> >> > >> > >> > > > it >> >> > >> > >> > > > > if >> >> > >> > >> > > > > > other people felt that it should be changed. >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > > 2. Move the interface out of scala to be a normal >> >> Java >> >> > >> > >> interface >> >> > >> > >> > > > > > > >> >> > >> > >> > > > > > > This breaks source compatibility but probably >> what we >> >> > >> should >> >> > >> > >> have >> >> > >> > >> > > > done >> >> > >> > >> > > > > > > originally I suspect. Probably there are few >> enough >> >> > >> > >> > implementations >> >> > >> > >> > > > of >> >> > >> > >> > > > > > this >> >> > >> > >> > > > > > > that it is better to just rip the bandaid off. >> >> > >> > >> > > > > > > >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > Can you please explain the motivation? It did come >> up >> >> in >> >> > >> > >> previous >> >> > >> > >> > > > > > discussions that some things like Operation and >> >> > ResourceType >> >> > >> > >> should >> >> > >> > >> > > be >> >> > >> > >> > > > in >> >> > >> > >> > > > > > the clients library, but not Authorizer itself. Are >> we >> >> > >> saying >> >> > >> > >> that >> >> > >> > >> > > any >> >> > >> > >> > > > > > pluggable interface should be in Java so that users >> can >> >> > >> > >> implement >> >> > >> > >> > it >> >> > >> > >> > > > > > without including the Scala library? >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > Grant, you originally suggested that some things >> would >> >> > have >> >> > >> to >> >> > >> > >> be >> >> > >> > >> > in >> >> > >> > >> > > > the >> >> > >> > >> > > > > > Java side as well. Can you please elaborate on this? >> >> > >> > >> > > > > > >> >> > >> > >> > > > > > Ismael >> >> > >> > >> > > > > > >> >> > >> > >> > > > > >> >> > >> > >> > > > >> >> > >> > >> > > > >> >> > >> > >> > > > >> >> > >> > >> > > > -- >> >> > >> > >> > > > Grant Henke >> >> > >> > >> > > > Software Engineer | Cloudera >> >> > >> > >> > > > gr...@cloudera.com <javascript:;> | twitter.com/gchenke >> | >> >> > >> > >> linkedin.com/in/granthenke >> >> > >> > >> > > > >> >> > >> > >> > > >> >> > >> > >> > >> >> > >> > >> > >> >> > >> > >> > >> >> > >> > >> > -- >> >> > >> > >> > Grant Henke >> >> > >> > >> > Software Engineer | Cloudera >> >> > >> > >> > gr...@cloudera.com <javascript:;> | twitter.com/gchenke | >> >> > >> linkedin.com/in/granthenke >> >> > >> > >> > >> >> > >> > >> >> >> > >> > > >> >> > >> > > >> >> > >> > > >> >> > >> > > -- >> >> > >> > > >> >> > >> > > Regards, >> >> > >> > > Ashish >> >> > >> > > >> >> > >> > >> >> > >> > >> >> > >> > >> >> > >> > -- >> >> > >> > >> >> > >> > Regards, >> >> > >> > Ashish >> >> > >> >> >> > > >> >> > >-- >> >> > > >> >> > >Regards, >> >> > >Ashish >> >> > >> >> > >> >> >> > >> > >> > >> >-- >> > >> >Regards, >> >Ashish >> >> > >-- >Ashish 🎤h
