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
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
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
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
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
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
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
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
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
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,
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
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
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
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
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
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
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
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
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
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`, `
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
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,
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
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
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
25 matches
Mail list logo