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> 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> 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> 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> > wrote: > > > > > > > Hi Jay, > > > > > > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps <j...@confluent.io> 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 | twitter.com/gchenke | linkedin.com/in/granthenke > > > -- Grant Henke Software Engineer | Cloudera gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
