kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randall Hauch <rha...@gmail.com>
Subject Re: [DISCUSS] KIP-458: Connector Client Config Override Policy
Date Tue, 07 May 2019 00:44:56 GMT
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 <mageshn@confluent.io>
wrote:

> 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 current behavior.
> 2. introduce `useOverride()` default method that returns true in the
> interface. The `Ignore` policy would then override it to return false.
> `Ignore` will also be the default policy.
>
> I personally prefer option #2, since it involves one less configuration but
> then I'm also open to the other option.
>
> Thanks,
> Magesh
>
> On Mon, May 6, 2019 at 5:29 PM Randall Hauch <rhauch@gmail.com> wrote:
>
> > 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.
> >
> > However, I still find it strange to have a "non-policy" be the default.
> So
> > either of these modifications to the current KIP would be fine with me:
> > 1) Add a `useOverride()` default method that returns true, but which the
> > None policy could override and return false; and keep the `validate(...)`
> > method as it is.
> > 2) Change the `validate(Map<...>) method to support a filtering pattern,
> > such as `Map<...> filterOverrides(Map<...> connectorClientOverrides)`
> >
> > The point is that the default is the name of a built-in policy.
> >
> > Also, one minor suggestion is to use the term "override" in the config
> > property (e.g., `connector.client.override.policy`) since that term is
> used
> > prevalently and matches the `producer.override`, `consumer.override`, and
> > `admin.override` prefixes.
> >
> > Thanks for working through this, Magesh.
> >
> > Randall
> >
> > On Mon, May 6, 2019 at 1:11 PM Magesh Nandakumar <mageshn@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 <
> mageshn@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 <rhauch@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 <chrise@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 <rhauch@gmail.com>
> wrote:
> > > >> >
> > > >> > > On Tue, Apr 30, 2019 at 4:20 PM Magesh Nandakumar <
> > > >> mageshn@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 <
> rhauch@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 <
> > > >> chrise@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 <
> > > >> > > > mageshn@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 <
> > > >> > chrise@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 <
> > > >> > > > > > mageshn@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 <
> > > >> > > > > chrise@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 <
> > > >> > > > > > > > mageshn@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 <
> > > >> > > > > > > > > mageshn@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
> <
> > > >> > > > > > > chrise@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 <
> > > >> > > > > > > > > > mageshn@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
> > > >> > > > > > > > > > > >> >
> > > >> > > > > > > > > > > >>
> > > >> > > > > > > > > > > >
> > > >> > > > > > > > > > >
> > > >> > > > > > > > > >
> > > >> > > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > >
> > > >> > > > > >
> > > >> > > > >
> > > >> > > >
> > > >> > >
> > > >> >
> > > >>
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message