Hey Colin, Thanks, I think continuing to bump the protocol makes sense. It's nice to be consistent with the other APIs. In the KIP, you have the following:
> If the client is newer than the broker, some of the fields may show up as UNKNOWN on the broker side. In this case, the filter will get an UnsupportedVersionException and the filter will not be applied. So the only case this would happen is if the client sent an invalid type for the request version, right? In that case, I would expect the client to raise an exception before sending the request when it sees that the broker does not support the needed request version. So maybe this should it be an invalid request error instead perhaps? Thanks, Jason On Wed, May 10, 2017 at 3:10 PM, Colin McCabe <cmcc...@apache.org> wrote: > Hi Jason, > > Thanks for taking a look. > > I think it's likely that we will bump the ApiVersion of the various > requests if we add a new resource type, operation type, or permission > type. Although it may not strictly be required, bumping the version > number makes it a little bit clearer that the protocol is evolving and > clarifies the relative age of the server and client. This would be > helpful in a debugging situation when using broker-api-versions.sh, or > similar, since it would clarify that the server does/does not support > some particular detail of ACLs. > > However, the v1 server will still need a way of responding to a v0 > ListAclRequest. In that case, any type not present in v0 will be mapped > to UNKNOWN, as required. This mapping could be done either on the > client or the server side... for simplicity's sake, it probably should > be done on the client side. > > The intention is to make it clear to the v0 client that "something is > going on" even if the v0 client can't see what the exact details are of > the v1 ACL. For example, the v0 client might see some ACLs on a > particular topic, but not be able to understand exactly what they are > (they are type UNKNOWN) until you upgrade your client. This would at > least give you some idea what was going on if a user was having trouble > doing something with that topic. There may be cases where we can't even > provide this much (for example, if we ever implement role-based access > control, the v0 client simply won't have any idea about it). But we are > doing what we can for compatibility. > > cheers, > Colin > > > On Wed, May 10, 2017, at 12:26, Jason Gustafson wrote: > > Hi Colin, > > > > Thanks for the KIP. Looks good overall. One thing I wasn't too clear > > about > > is whether new resource types, operations, or permissions require a > > version > > bump for the three new request types. On reading the proposal, it sort of > > sounds like the intent is not to bump the versions and let the new type > > be > > mapped to "unknown" on clients/brokers that don't support it. Is that > > correct? > > > > Thanks, > > Jason > > > > On Wed, May 10, 2017 at 8:24 AM, Ismael Juma <ism...@juma.me.uk> wrote: > > > > > Thanks for the KIP, +1(binding) from me. Colin, it may make sense to > start > > > a new thread for this as GMail is currently hiding it in the middle of > the > > > discuss thread. Please mention the date the thread was started along > with > > > the 3 +1s (1 binding) so far. > > > > > > Ismael > > > > > > On Sat, Apr 29, 2017 at 1:09 AM, Colin McCabe <cmcc...@apache.org> > wrote: > > > > > > > Hi all, > > > > > > > > I'd like to start the voting for KIP-140: Add administrative RPCs for > > > > adding, deleting, and listing ACLs. This provides an API for adding, > > > > deleting, and listing the access control lists (ACLs) which are used > to > > > > control access on Kafka topics and brokers. > > > > > > > > The wiki page is here: > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > 140%3A+Add+administrative+RPCs+for+adding%2C+deleting% > > > 2C+and+listing+ACLs > > > > > > > > The previous [DISCUSS] thread: > > > > https://www.mail-archive.com/dev@kafka.apache.org/msg70858.html > > > > > > > > cheers, > > > > Colin > > > > > > > >
