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 those are additional connector properties to help resolve the Kafka cluster other than the one Connect framework knows about. Whereas, the proposal in KIP-458 applies producer policies for SinkConnectors and consumer policies SourceConnectors. So, from what I understand this new policy should work without any issues even for Mirror Maker 2.0. 2. I have updated the KIP to use a default value of null and use that to determine if we need to ignore overrides. 3. I would still prefer to keep the special PrincipalConnectorClientConfigPolicy since that is one of the most common use cases one would choose to use this feature. If we make it a general case, that would involve users requiring to add additional configuration and they might require well more than just the list of configs but might also want some restriction on values. If the concern is about users wanting principal and also other configs, it would still be possible by means of a custom implementation. As is, I would prefer to keep the proposal to be the same for this. Let me know your thoughts. Thanks, Magesh On Mon, Apr 22, 2019 at 3:44 PM Chris Egerton <chr...@confluent.io> wrote: > 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 upcoming Mirror Maker 2.0 > (KIP-382) that already support properties with those prefixes? Would it be > possible for a user to configure MM2 with those properties without them > being interpreted as Connect client overrides, without isolating MM2 onto > its own cluster and using the IgnoreConnectorClientConfigPolicy policy? > 2. Is the IgnoreConnectorClientConfigPolicy class necessary? The default > for the connector.client.config.policy property could simply be null > instead of a new policy that, as far as I can tell, isn't an actual policy > in that its validate(...) method is never invoked and instead represents a > special case to the Connect framework that says "Drop all overrides and > never use me". > 3. The PrincipalConnectorClientConfigPolicy seems like a specific instance > of a more general use case: allow exactly a small set of overrides and no > others. Why not generalize here and create a policy that accepts a list of > allowed overrides during configuration? > > Thanks again for the KIP. > > Cheers, > > Chris > > On Fri, Apr 19, 2019 at 2:53 PM Magesh Nandakumar <mage...@confluent.io> > wrote: > > > 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+Override+Policy > > . > > > > Looking forward for the discussion on the KIP and all of your thoughts & > > feedback on this enhancement to Connect. > > > > Thanks, > > Magesh > > >
