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 the default policy. Currently the Ignore policy "Behavior" just mentions that it's the current behavior, but I think it would help that it is described as the default for the property.
Otherwise, this looks good to me. Best regards, Randall On Mon, May 6, 2019 at 8:12 PM Magesh Nandakumar <mage...@confluent.io> wrote: > 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 Karantasis < > konstant...@confluent.io> wrote: > > > 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 KIP says: 'In addition to the default implementation, ..." > > but this is not very accurate because there is no concrete default > > implementation. Just special handling of 'null' in > > 'connector.client.config.policy' > > > > Regarding passing the overrides to the connector 'configure' method, I > feel > > it wouldn't hurt to pass them, but I also agree that leaving this out at > > the moment is the safest option. > > > > Since the interfaces and classes are listed in the KIP, I'd like to note > > that Class is used as a raw type in field and return value declarations. > We > > should use the generic type instead. > > > > Thanks for this improvement proposal! > > Konstantine > > > > On Mon, May 6, 2019 at 11:11 AM Magesh Nandakumar <mage...@confluent.io> > > wrote: > > > > > 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 < > mage...@confluent.io> > > > wrote: > > > > > > > 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 > > > > for the default `null` I would prefer to fall back to using `Ignore` > > > which > > > > is a misnomer to the interface spec but still gets the job done via > > > > instanceOf checks. The other options I could think of are as below:- > > > > > > > > - have an enforcePolicy() method in the interface which by default > > > > returns true and the Ignore implementation could return false > > > > - introduce another worker config allow.connector.config.overrides > > > > with a default value of false and the default policy can be None > > > > > > > > Let me know what you think. > > > > > > > > Thanks > > > > Magesh > > > > > > > > On Tue, Apr 30, 2019 at 6:52 PM Randall Hauch <rha...@gmail.com> > > wrote: > > > > > > > >> 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 "validate" method take the overrides that were supplied and > > > >> return > > > >> the overrides that should be passed to the connector, yet still > > throwing > > > >> an > > > >> exception if any supplied overrides are not allowed. Then the > > different > > > >> policy implementations might be: > > > >> > > > >> - Ignore (default) - returns all supplied override properties > > > >> - None - throws exception if any override properties are > supplied; > > > >> always returns empty map if no overrides are provided > > > >> - Principal - throws exception if other override properties are > > > >> provided, but returns an empty map (since no properties should be > > > >> passed to > > > >> the connector) > > > >> - All - returns all provided override properties > > > >> > > > >> All override properties defined on the connector configuration would > > be > > > >> passed to the policy for validation, and assuming there's no error > all > > > of > > > >> these overrides would be used in the producer/consumer/admin client. > > The > > > >> result of the policy call, however, is used to determine which of > > these > > > >> overrides are passed to the connector. > > > >> > > > >> This approach means that all behaviors can be implemented through a > > > policy > > > >> class, including the defaults. It also gives a bit more control to > > > custom > > > >> policies, should that be warranted. For example, validating the > > provided > > > >> client overrides but passing all such override properties to the > > > >> connector, > > > >> which as I stated earlier is something I think connectors likely > don't > > > >> look > > > >> for. > > > >> > > > >> Thoughts? > > > >> > > > >> Randall > > > >> > > > >> On Tue, Apr 30, 2019 at 6:07 PM Chris Egerton <chr...@confluent.io> > > > >> wrote: > > > >> > > > >> > 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 > > > >> operator) > > > >> > and, if present, cause all would-be overrides to be ignored. > > > >> > > > > >> > I thought this may be confusing to people who may see that > behavior > > > and > > > >> > wonder how to recreate it themselves, so I suggested leaving that > > > policy > > > >> > out and replace it with a check to see if a policy was specified > at > > > all. > > > >> > > > > >> > Would be interested in your thoughts on this, especially if > there's > > an > > > >> > alternative that hasn't been proposed yet. > > > >> > > > > >> > Cheers, > > > >> > > > > >> > Chris > > > >> > > > > >> > On Tue, Apr 30, 2019, 18:01 Randall Hauch <rha...@gmail.com> > wrote: > > > >> > > > > >> > > On Tue, Apr 30, 2019 at 4:20 PM Magesh Nandakumar < > > > >> mage...@confluent.io> > > > >> > > 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 connector. But this is a > specific > > > case > > > >> > and > > > >> > > we > > > >> > > > could do either of the following > > > >> > > > > > > >> > > > > > > >> > > > - don't pass any configs with these prefixes to the > > > >> ConnectorConfig > > > >> > > > instance that's passed in the startConnector > > > >> > > > - allow policies as to whether the configurations with the > > > >> prefixes > > > >> > > > should be made available to the connector or not. Should > this > > > >> also > > > >> > > > define a > > > >> > > > list of configurations? > > > >> > > > > > > >> > > > I personally prefer not passing the configs to Connector since > > > >> that's > > > >> > > > simple, straight forward and don't see a reason for the > > connector > > > to > > > >> > > access > > > >> > > > those. > > > >> > > > > > > >> > > > > > >> > > I agree that these override properties should be effectively new > > > >> > > properties, in which case I'd also prefer that they be removed > > from > > > >> the > > > >> > > configuration before it is passed to the connector. Yes, it is > > > >> *possible* > > > >> > > that an existing connector happened to use connector config > > > properties > > > >> > with > > > >> > > these prefixes, but it's seems pretty unlikely. > > > >> > > > > > >> > > I'd love to hear whether other people agree. > > > >> > > > > > >> > > > > > >> > > > > > > >> > > > For the second point, None - doesn't allow overrides and the > > > >> default > > > >> > > > policy is null. We preserve backward compatibility when no > > policy > > > is > > > >> > > > configured. Let me know if that's not clear in the KIP. > > > >> > > > > > > >> > > > > > >> > > Why not have a default policy (rather than null) that implements > > the > > > >> > > backward-compatible behavior? It seems strange to have null be > the > > > >> > default > > > >> > > and for non-policy to allow anything. > > > >> > > > > > >> > > > > > >> > > > > > > >> > > > Thanks, > > > >> > > > Magesh > > > >> > > > > > > >> > > > On Mon, Apr 29, 2019 at 4:07 PM Randall Hauch < > rha...@gmail.com > > > > > > >> > wrote: > > > >> > > > > > > >> > > > > 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 on these configurations to be available. The > > > >> > overrides > > > >> > > > are > > > >> > > > > to be used purely from an operational perspective. > > > >> > > > > > > > >> > > > > > > > >> > > > > Does this mean that any such properties are visible to > > > >> connectors, or > > > >> > > > will > > > >> > > > > they be hidden to connectors? Currently no connectors have > > > access > > > >> to > > > >> > > such > > > >> > > > > client properties, and users are unlike to just put them > into > > a > > > >> > > connector > > > >> > > > > configuration unnecessarily. A connector implementation > could > > > have > > > >> > > > defined > > > >> > > > > such properties as normal connector-specific properties, in > > > which > > > >> > case > > > >> > > > they > > > >> > > > > are required, but is that likely given the log prefixes? One > > > >> concern > > > >> > > > that I > > > >> > > > > have is that this might allow connector implementations > start > > > >> > > attempting > > > >> > > > to > > > >> > > > > circumvent the Connect API if these properties are included. > > > >> > > > > > > > >> > > > > Second, does the None policy allow but ignore these > additional > > > >> > > properties > > > >> > > > > (e.g., "validate(...)" is simply a no-op)? Or does the None > > > policy > > > >> > fail > > > >> > > > if > > > >> > > > > any client overrides are specified? The former seems more in > > > line > > > >> > with > > > >> > > > the > > > >> > > > > current behavior, whereas the "disallows" policy seems > useful > > > but > > > >> not > > > >> > > > > exactly backward compatible. Should we also offer a > "Disallow" > > > >> > policy? > > > >> > > In > > > >> > > > > fact, should the policies be named "Ignore" (default), > > > "Disallow", > > > >> > > > > "Prinicipal", and "All"? > > > >> > > > > > > > >> > > > > Otherwise, I like the idea of this. There have been several > > > >> requests > > > >> > > over > > > >> > > > > the past year or two for adding subsets of this > functionality. > > > >> Might > > > >> > be > > > >> > > > > good to find and list all of the related KAFKA issues. > > > >> > > > > > > > >> > > > > Randall > > > >> > > > > > > > >> > > > > On Fri, Apr 26, 2019 at 4:04 PM Chris Egerton < > > > >> chr...@confluent.io> > > > >> > > > wrote: > > > >> > > > > > > > >> > > > > > 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 < > > > >> > > > mage...@confluent.io> > > > >> > > > > > wrote: > > > >> > > > > > > > > >> > > > > > > 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 < > > > >> > chr...@confluent.io > > > >> > > > > > > >> > > > > > wrote: > > > >> > > > > > > > > > >> > > > > > > > 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 > > > >> > > > > "override" > > > >> > > > > > > may > > > >> > > > > > > > shed a little light on how the properties for these > > > clients > > > >> are > > > >> > > > > > computed > > > >> > > > > > > > and help make the learning curve a little gentler on > new > > > >> devs > > > >> > and > > > >> > > > > > users. > > > >> > > > > > > > > > > >> > > > > > > > Regardless, I think the larger issue of conflicts with > > > >> existing > > > >> > > > > > > properties > > > >> > > > > > > > (both in MM2 and potentially other connectors) has > been > > > >> > > > > satisfactorily > > > >> > > > > > > > addressed, so I'm happy. > > > >> > > > > > > > > > > >> > > > > > > > Cheers, > > > >> > > > > > > > > > > >> > > > > > > > Chris > > > >> > > > > > > > > > > >> > > > > > > > On Wed, Apr 24, 2019 at 11:14 AM Magesh Nandakumar < > > > >> > > > > > mage...@confluent.io > > > >> > > > > > > > > > > >> > > > > > > > wrote: > > > >> > > > > > > > > > > >> > > > > > > > > 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`, `connector.consumer` > and > > > >> > > > > > > `connector.admin` > > > >> > > > > > > > - > > > >> > > > > > > > > provides better context that its connector specific > > > >> property > > > >> > > > > > > > > 3. Use `producer.override`, '`consumer.override` > and > > > >> > > > > > `admin.override` > > > >> > > > > > > - > > > >> > > > > > > > > provides better clarity that these are overrides. > > > >> > > > > > > > > > > > >> > > > > > > > > I don't have a strong opinion in choosing between #2 > > and > > > >> #3. > > > >> > > Let > > > >> > > > me > > > >> > > > > > > > > know what you think. > > > >> > > > > > > > > > > > >> > > > > > > > > Thanks > > > >> > > > > > > > > Magesh > > > >> > > > > > > > > > > > >> > > > > > > > > On Wed, Apr 24, 2019 at 10:25 AM Chris Egerton < > > > >> > > > > chr...@confluent.io> > > > >> > > > > > > > > wrote: > > > >> > > > > > > > > > > > >> > > > > > > > > > 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 > > > scenario > > > >> > like > > > >> > > > > this: > > > >> > > > > > a > > > >> > > > > > > > > > Connect worker is configured to use the > > > >> > > > > > > > > > PrincipalConnectorClientConfigPolicy, someone > tries > > to > > > >> > start > > > >> > > an > > > >> > > > > > > > instance > > > >> > > > > > > > > of > > > >> > > > > > > > > > an MM2 sink with "admin." properties beyond just > > > >> > > > > > > > > "admin.sasl.jaas.config", > > > >> > > > > > > > > > and gets rejected because those properties are > then > > > >> > > interpreted > > > >> > > > > by > > > >> > > > > > > the > > > >> > > > > > > > > > worker as overrides for the AdminClient it uses to > > > >> manage > > > >> > the > > > >> > > > > DLQ. > > > >> > > > > > > > > > 2. (LGTM) > > > >> > > > > > > > > > 3. I'm convinced by this, as long as nobody else > > > >> > identifies a > > > >> > > > > > common > > > >> > > > > > > > use > > > >> > > > > > > > > > case that would involve a similar client config > > policy > > > >> > > > > > implementation > > > >> > > > > > > > > that > > > >> > > > > > > > > > would be limited to a small set of whitelisted > > > configs. > > > >> For > > > >> > > now > > > >> > > > > > > keeping > > > >> > > > > > > > > the > > > >> > > > > > > > > > PrincipalConnectorClientConfigPolicy sounds fine > to > > > me. > > > >> > > > > > > > > > > > > >> > > > > > > > > > Cheers, > > > >> > > > > > > > > > > > > >> > > > > > > > > > Chris > > > >> > > > > > > > > > > > > >> > > > > > > > > > On Tue, Apr 23, 2019 at 10:30 PM Magesh > Nandakumar < > > > >> > > > > > > > mage...@confluent.io > > > >> > > > > > > > > > > > > >> > > > > > > > > > wrote: > > > >> > > > > > > > > > > > > >> > > > > > > > > > > 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 > > > >> > > > > > > > > > > >> > > > > >> > > > > > > > > > > >> > > > >> > > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > > > > > > > >
