Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi, Since there were no additional on this KIP, I'd like to restart the vote tomorrow. If anyone has comments, please do address them. Thanks, Viktor On Mon, Feb 19, 2018 at 9:44 AM, Viktor Somogyi wrote: > Hi Rajini, > > Thanks for the feedback, I've applied your

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Rajini, Thanks for the feedback, I've applied your points. Viktor On Wed, Feb 7, 2018 at 7:22 PM, Rajini Sivaram wrote: > Hi Viktor, > > Thanks for the updates. Looks good, just a few minor comments: > >1. AdminOperation - could be AlterOperation since it is

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Viktor, Thanks for the updates. Looks good, just a few minor comments: 1. AdminOperation - could be AlterOperation since it is only applied to 'Alter'? 2. Don't think we need `Unknown` type to process old requests. We can use `Set` as the default for alter requests with version 0.

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Rajini, I think it makes sense absolutely and even we could do it for AlterQuotas as we will have the same problem there. Updated my KIP to reflect these changes: - proposed

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Viktor, While implementing KAFKA-6494, I realised that there is a mismatch between the --alter command of ConfigCommand and AlterConfigs request. ConfigCommand uses --add-config and --delete-config to make incremental updates. --add-config reads all the configs from ZooKeeper and adds the

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Rajini, Ok, I think I got you. I wasn't calculating with the fact that the parent might not be set, therefore it could be a default user as well or even the default client if nothing else is set (supposing we're talking about the example). So if I'm correct, the quota will be

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Viktor, Thanks for the updates. *QuotaSource* currently has *Self/Default/Parent*. Not sure if that is sufficient. For the entity , quota could be used from any of these configs: 1. /config/users//clients/ 2. /config/users//clients/ 3. /config/users/ 4.

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Rajini, I have updated the KIP as agreed. Could you please have a second look at it? I have also added a section about SCRAM: "To enable describing and altering SCRAM credentials we will use the DescribeConfigs and AlterConfigs protocols. There are no changes in the protocol's structure but we

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

3. Ok, I'll remove this from the KIP for now and perhaps add a future considerations section with the idea. 9. Ok, I can do that. On Thu, Jan 18, 2018 at 11:18 AM, Rajini Sivaram wrote: > Hi Viktor, > > 3. Agree that it would be better to use something like

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Viktor, 3. Agree that it would be better to use something like ConfigEntityList rather than ListQuotas. But I would leave it out for now since we are so close to KIP freeze. We can introduce it later if required. Earlier, I was thinking that if we just wanted to get a list of entities without

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Rajini, 1. Yes, --adminclient.config it is. I missed that, sorry :) 3. Indeed SCRAM in this case can raise complications. Since we'd like to handle altering SCRAM credentials via AlterConfigs, I think we should use DescribeConfigs to describe them. That is, to describe all the entities of a

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

Hi Viktor, Thank you for the responses. 1. ConsoleProducer uses *--producer.config --producer-property key=value*, ConsoleConsumer uses* --consumer.config --consumer-property key=value*, so perhaps we should use *--adminclient.config *rather than *--config.properties*? 3. The one difference

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

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

Re: [DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

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

[DISCUSS] KIP-248 Create New ConfigCommand That Uses The New AdminClient

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