Hi Rajini,
1 and 2: corrected it in my code. So there will be 3 properties in this
group: --bootstrap-server, --config.properties and --adminclient-property
(following the conventions established elsewhere, like the
console-producer).
3: Let me explain the reason for ListQuotas. In the current version of
kafka-configs you can do this:
bin/kafka-configs.sh --zookeeper localhost:2181 --describe --entity-type
users
And this will return you all the configs for users under /config/users
znode. In that command you have direct access to zookeeper, so you can
instantly do an iteration through the znode. Therefore I looked at other
protocols (ListAcls ListGroups, ListTopics) and thought it would be aligned
with those if I separated off listing. This has the pros of being able to
return a list of entities or fine tuning permissions (you might have users
who don't have to know other users' quota settings). Once the list of
resources returned, the user can initiate a bulk describe.
Of course the cons of having ListQuotas as a separate protocol is that it
might do something too simple for a protocol and actually as you say it
might be implemented with DescribeQuotasOptions perhaps by only using an
extra flag in the DescribeQuotas protocol (like "describe_all").
Do you think it would be better to add an option to "describe all"? Also of
course the response would be "asymmetric" to the request in this case
meaning that I send one resource and might get back more. One of my reasons
of implementing this "list then describe" way of doing things was to be
aligned with DescribeConfigs (as that is also symmetric similarly).
4. OK, I think I can do that, it makes sense.
5. Sure, I can do that. In fact I started with this but then reverted as I
didn't know if it's really planned to have more levels.
6. OK, will remove those.
7. Well, at this point if you specify (userA, client1) it will simply get's
the znode's data at /config/users/userA/clients/client1 . If there is no
such client it returns empty. This now functionally compatible with the
current ConfigCommand. However what you're saying I think makes sense,
meaning that we want to return what will actually be applied if the exact
mapping doesn't exist (and may tell the user that for instance "there is no
client1 for userA so I returned that will be applied").
8. Certainly, I can include that as well. Just to confirm, are you thinking
about something like this?
bin/kafka-configs.sh --zookeeper localhost:2181 --alter --add-config
'SCRAM-SHA-256=[iterations=8192,password=alice-secret],SCRAM-SHA-512=[password=alice-secret]'
--entity-type users --entity-name alice
Which will result in:
get /config/users/alice
{"version":1,"config":{"SCRAM-SHA-512":"salt=some_salt,stored_key=some_key,server_key=some_server_key,iterations=4096","SCRAM-SHA-256":"salt=some_salt,stored_key=some_key_2,server_key=some_server_key_2,iterations=8192"}}
Regards,
Viktor
On Tue, Jan 16, 2018 at 12:33 PM, Rajini Sivaram <rajinisiva...@gmail.com>
wrote:
> Hi Viktor,
>
> Thank you for the KIP. It is looking good. A few comments:
>
> 1. --bootstrap-server option: "*Help Message*" uses --bootstrap-servers. I
> think other tools use the singular form even though it should probably have
> been plural to start with. Can we use* --bootstrap-server* for consistency?
>
> 2. ConsoleProducer/ConsoleConsumer etc. have two ways of providing client
> properties: *--producer.config <configFile>* and *--producer.property
> key=value*. Can we do the same here since it is easier for scripts to pass
> in property on the command line sometimes rather than a file. Perhaps
> *--adminclient.config
> *and *--adminclient.property* or something along those lines?
>
> 3. Not sure if ListQuotas is useful. For various other entities, we just
> have a Describe request. Wouldn't that be sufficient? If we did want a less
> detailed version, we could have options for the describe request.
>
> 4. We use "<default>" internally to store default quotas and other
> defaults. But I don't think we should externalise that string. We use empty
> string elsewhere for indicating default, we can do the same here. And we
> use STRING rather than NULLABLE_STRING in describe configs etc. So we
> should probably do the same for quotas.
>
> 5. At the moment, we have two levels of quotas and you can define <user,
> clientId> quotas. We may want to add more levels in the future, eg. <group,
> user, clientId>. It would be more flexible to specify an array of entities
> rather than a single child entity. (e.g resource => [quota_entity],
> quota_entity => entity_type entity_name).
>
> 6. In DescribeQuotasResponse, we don't need the flags read_only or
> is_sensitve since they will always be false for quotas.
>
> 7. What will DescribeQuotas(userA, client1) actually return? Will it return
> the quota defined for (userA, client1)? Or will it return the quota that
> will be applied to (userA, client1) - i.e. if there is no quota defined for
> (userA, client1), but there is a quota defined for userA, will it return
> userA quota since that will be the quota applied to (userA, client1)?
>
> 8. I think at the moment, AdminClient doesn't have an API for creating
> SCRAM credentials (ConfigCommand is used for this). I think the existing
> describe/alterConfigs API can be used if we add a resource type USER. If
> this will be done in the same JIRA, can we include it in the KIP?
>
> Regards,
>
> Rajini
>
>
> On Fri, Jan 12, 2018 at 11:11 AM, Viktor Somogyi <viktorsomo...@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I have submitted KIP-248 that details how can kafka-configs.sh could
> > utilize KafkaAdminClient through a new ConfigCommand class in the tools
> > module:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+
> > Create+New+ConfigCommand+That+Uses+The+New+AdminClient
> >
> > This KIP proposes to add 3 new wire protocols (listing quota configs,
> > describing quota configs and altering quota configs), a new class called
> > ConfigCommand in tools which would be the main entry point for
> > kafka-configs.sh and a way to deprecate the current Scala ConfigCommand.
> >
> > I'd be happy to receive some feedback about the proposal.
> >
> > Thank you & regards,
> > Viktor
> >
>
