It is small, but breaks binary compatibility. Ismael
On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke <ghe...@cloudera.com> 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> 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 >
