Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-07 Thread Magesh Nandakumar
I have addressed all the outstanding discussion points and I believe we have consensus on the design. Thanks, everyone for the feedback. I will start a VOTE thread on this KIP. On Mon, May 6, 2019 at 10:13 PM Magesh Nandakumar wrote: > Randall, > > Thanks a lot for the suggestions. I have incorp

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Magesh Nandakumar
Randall, Thanks a lot for the suggestions. I have incorporated the comments in the KIP. Thanks, Magesh On Mon, May 6, 2019 at 6:52 PM Randall Hauch wrote: > Thanks, Magesh. I do have a few pretty minor suggestions. > > 1) Define a bit more clearly in the "Proposed Changes" whether the configs

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Randall Hauch
Thanks, Magesh. I do have a few pretty minor suggestions. 1) Define a bit more clearly in the "Proposed Changes" whether the configs passed to the validate method via the ConnectorClientConfigRequest object have or do not have the prefixes. 2) Specify more clearly in (or around) the table which is

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Magesh Nandakumar
Hi Randall, I have incorporated your comments and updated the KIP with the following 1. update the config key to be `connector.client.config.override.policy` 2. update the interface name to be `ConnectorClientConfigOverridePolicy`. Updated the implementation names also to match the n

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Magesh Nandakumar
Konstantine, Thanks a lot for your feedback on the KIP. I have incorporated the feedback using generics for Class. I have also updated the KIP to handle the default value per Randall's suggestion. Let me know if you have any questions. Thanks, Magesh On Mon, May 6, 2019 at 1:58 PM Konstantine K

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Randall Hauch
That's fine. I don't think there's much difference between the two option anyway. I'm looking forward to the updated KIP. On Mon, May 6, 2019 at 7:38 PM Magesh Nandakumar wrote: > Randall, > > Thanks a lot for your feedback. > > If I understand it correctly, we could do one of the following, rig

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Magesh Nandakumar
Randall, Thanks a lot for your feedback. If I understand it correctly, we could do one of the following, right? 1. introduce a new config `allow.client.config.overrides` with a default value of false. The default value for the policy would be `None`. So by default, we will still preserve the cur

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Randall Hauch
I actually like a separate config for whether to pass or filter client override properties to the connector. I generally dislike adding more properties, but in this case it keeps the two orthogonal behaviors independent and reduces the need to implement policies that cover all permutations. Howeve

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Konstantine Karantasis
Thanks for the KIP Magesh, it's quite useful towards the goals for more general multi-tenancy in Connect. Couple of comments from me too: I think the fact that the default policy is 'null' (no implementation) should be mentioned on the table next to the available implementations. Currently the KI

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-05-06 Thread Magesh Nandakumar
Randall, I was wondering if you had any thoughts on the above alternatives to deal with a default policy. If it's possible, I would like to finalize the discussions and start a vote. Let me know your thoughts. Thanks, Magesh On Tue, Apr 30, 2019 at 8:46 PM Magesh Nandakumar wrote: > Randall,

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-30 Thread Magesh Nandakumar
Randall, The approach to return the to override configs could possibly make it cumbersome to implement a custom policy. This is a new configuration and if you don't explicitly set it the existing behavior remains as-is. Like Chris, I also preferred this approach for the sake of simplicity. If not

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-30 Thread Randall Hauch
Thanks, Chris. I still think it's strange to have a non-policy, since there's now special behavior for when the policy is not specified. Perhaps the inability for a policy implementation to represent the existing behavior suggests that the policy interface isn't quite right. Could the policy's "va

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-30 Thread Chris Egerton
Randall, The special behavior for null was my suggestion. There is no implementation of the proposed interface that causes client overrides to be ignored, so the original idea was to have a special implementation that would be checked for by the Connect framework (probably via the instanceof opera

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-30 Thread Randall Hauch
On Tue, Apr 30, 2019 at 4:20 PM Magesh Nandakumar wrote: > Randall, > > Thanks a lot for your feedback. > > You bring up an interesting point regarding the overrides being available > to the connectors. Today everything that is specified in the config while > creating is available for the connect

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-30 Thread Magesh Nandakumar
Randall, Thanks a lot for your feedback. You bring up an interesting point regarding the overrides being available to the connectors. Today everything that is specified in the config while creating is available for the connector. But this is a specific case and we could do either of the following

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-29 Thread Randall Hauch
Per the proposal, a connector configuration can define one or more properties that begin with any of the three prefixes: "producer.override.", "consumer.override.", and "admin.override.". The proposal states: Since the users can specify any of these policies, the connectors itself should not rely

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-26 Thread Chris Egerton
Hi Magesh, Changes look good to me! Excited to see this happen, hope the KIP passes :) Cheers, Chris On Fri, Apr 26, 2019 at 1:44 PM Magesh Nandakumar wrote: > Hi Chris, > > I have updated the KIP to reflect the changes that we discussed for the > prefix. Thanks for all your inputs. > > Thank

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-26 Thread Magesh Nandakumar
Hi Chris, I have updated the KIP to reflect the changes that we discussed for the prefix. Thanks for all your inputs. Thanks, Magesh On Thu, Apr 25, 2019 at 2:18 PM Chris Egerton wrote: > Hi Magesh, > > Agreed that we should avoid `dlq.admin`. I also don't have a strong opinion > between `conn

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-25 Thread Chris Egerton
Hi Magesh, Agreed that we should avoid `dlq.admin`. I also don't have a strong opinion between `connector.` and `.override`, but I have a slight inclination toward `.override` since `connector.` feels a little redundant given that the whole configuration is for the connector and the use of "overri

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-24 Thread Magesh Nandakumar
HI Chrise, You are right about the "admin." prefix creating conflicts. Here are few options that I can think of 1. Use `dlq.admin` since admin client is used only for DLQ. But this might not really be the case in the future. So, we should possibly drop this idea :) 2. Use `connector.producer`, `

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-24 Thread Chris Egerton
Hi Magesh, Next round :) 1. It looks like MM2 will also support "admin." properties that affect AdminClients it creates and uses, which IIUC is the same prefix name to be used for managing the DLQ for sink connectors in this KIP. Doesn't that still leave room for conflict? I'm imagining a scenari

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-23 Thread Magesh Nandakumar
Hi all, I also have a draft implementation of the KIP https://github.com/apache/kafka/pull/6624. I would still need to include more tests and docs but I thought it would be useful to have this for the KIP discussion. Looking forward to all of your valuable feedback. Thanks Magesh On Tue, Apr 23,

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-23 Thread Magesh Nandakumar
Chrise, Thanks a lot for your feedback. I will address them in order of your questions/comments. 1. Thanks for bringing this to my attention about KIP-382. I had a closer look at the KIP and IIUC, the KIP allows `consumer.` prefix for SourceConnector and producer. prefix for SinkConnector since t

Re: [DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-22 Thread Chris Egerton
Hi Magesh, This is an exciting KIP! I have a few questions/comments but overall I like the direction it's headed in and hope to see it included in the Connect framework soon. 1. With the proposed "consumer.", "producer.", and "admin." prefixes, how will this interact with connectors such as the u

[DISCUSS] KIP-458: Connector Client Config Override Policy

2019-04-19 Thread Magesh Nandakumar
Hi all, I've posted "KIP-458: Connector Client Config Override Policy", which allows users to override the connector client configurations based on a policy defined by the administrator. The KIP can be found at https://cwiki.apache.org/confluence/display/KAFKA/KIP-458%3A+Connector+Client+Config+O