kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ismael Juma <ism...@juma.me.uk>
Subject Re: [DISCUSS] KIP-140: Add administrative RPCs for adding, deleting, and listing ACLs
Date Mon, 24 Apr 2017 11:36:57 GMT
Thanks Colin. A few quick comments:

1. Is there a reason why AddAclsRequest must be sent to the controller
broker? When this was discussed previously, Jun suggested that such a
restriction may not be necessary for this request.

2. Other protocol APIs use the Delete prefix instead of Remove. Is there a
reason to deviate here? If there's a good reason to do so, we have to fix
the one mention of DeleteAcls in the proposal.

3. Do you mean "null" in the following sentence? "Note that an argument of
"none" is different than a wildcard argument."

4. How will the non-zero top-level error code be computed? A bit more
detail on this would be helpful. As a general rule, most batch protocol
APIs don't have a top level error code because errors are usually at the
batch element level. This has proved to be a bit of an issue in cases where
we want to return a generic error code (e.g. InvalidProtocolVersion). Also,
V2 of OffsetFetch has a top-level error code[1]. However, the OffsetFetch
behaviour is that we either use the top-level error code or the partition
level error codes, which is different than what is being suggested here.

5. Nit: In other requests we used `error_message` instead of `error_string`.

6. Regarding the migration plan, it's worth making it clear that the CLI
transition is not part of this KIP.

7. Regarding the forward compatibility point, we could potentially use
enums with UNKNOWN element? This pattern has been used elsewhere in Kafka
and it would be good to compare it with the proposed solution. An important
question is what can users do with UNKNOWN elements. If the assumption is
that users ignore them, then it seems like the enum approach may be good
enough.

Thanks,
Ismael

[1]
https://cwiki.apache.org/confluence/display/KAFKA/KIP-88%3A+OffsetFetch+Protocol+Update


On Fri, Apr 21, 2017 at 9:27 PM, Colin McCabe <cmccabe@apache.org> wrote:

> Hi all,
>
> As part of the AdminClient work, we would like to add methods for
> adding, deleting, and listing access control lists (ACLs).  I wrote up a
> KIP to discuss implementing requests for those operations, as well as
> AdminClient APIs.  Take a look at:
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 140%3A+Add+administrative+RPCs+for+adding%2C+deleting%2C+and+listing+ACLs
>
> regards,
> Colin
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message