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, 2019 at 10:27 PM Magesh Nandakumar <mage...@confluent.io> wrote: > 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 >> > >> >
