Hi Chris,
Thanks for the explanation. Since there are workarounds and we are not
making it any worse, this should be fine.
Regards,
Rajini
On Wed, May 8, 2019 at 8:06 PM Chris Egerton wrote:
> Hi Rajini,
>
> That was an initial concern of mine as well but I think we should be fine.
> Connect
Hi Rajini,
That was an initial concern of mine as well but I think we should be fine.
Connect REST extensions are already capable of intercepting requests that
contain new connector configurations, through POST calls to the /connectors
endpoint and PUT calls to /connectors//config. The additional
Hi Chris,
Thanks for the KIP, looks good. I have just one question. Can `
ConnectClusterState#connectorConfig()` return any sensitive configs like
passwords?
Thanks,
Rajini
On Wed, May 8, 2019 at 1:30 AM Chris Egerton wrote:
> Hi all,
>
> Now that KAFKA-8304 (https://issues.apache.org/jira/b
Hi all,
Now that KAFKA-8304 (https://issues.apache.org/jira/browse/KAFKA-8304),
which was a blocker, has been addressed, I've published a PR for these
changes: https://github.com/apache/kafka/pull/6584
Thanks to everyone who's voted so far! If anyone else is interested, the
voting thread can be
Hi Konstantine,
I've updated the KIP to add default method implementations to the list of
rejected alternatives. Technically this makes the changes in the KIP
backwards incompatible, but I think I agree that for the majority of cases
where it would even be an issue a compile-time error is likely t
Hi Chris,
Thanks for considering my suggestion regarding default implementations for
the new methods.
However, given that these implementations don't seem to have sane defaults
and throw UnsupportedOperationException, I think we'll be better without
defaults.
Seems that a compile time error is pre
Thanks a lot, Chris. The KIP looks good to me.
On Thu, Apr 25, 2019 at 7:35 PM Chris Egerton wrote:
> Hi Magesh,
>
> Sounds good; I've updated the KIP to make ConnectClusterDetails an
> interface. If we want to leave the door open to expand it in the future it
> definitely makes sense to treat i
Hi Magesh,
Sounds good; I've updated the KIP to make ConnectClusterDetails an
interface. If we want to leave the door open to expand it in the future it
definitely makes sense to treat it similarly to how we're treating the
ConnectClusterState interface now.
Thanks,
Chris
On Thu, Apr 25, 2019 a
HI Chrise,
Overall it looks good to me. Just one last comment - I was wondering if
ConnectClusterDetail should be an interface just like ConnectClusterState.
Thanks,
Magesh
On Thu, Apr 25, 2019 at 3:54 PM Chris Egerton wrote:
> Hi Magesh,
>
> Expanding the type we use to convey cluster metadat
Hi Magesh,
Expanding the type we use to convey cluster metadata from just a Kafka
cluster ID string to its own class seems like a good idea for the sake of
forwards compatibility, but I'm still not sure what the gains of including
the cluster group ID would be--it's a simple map lookup away in the
Hi Chris,
Instead of calling it ConnectClusterId, perhaps call it
ConnectClusterDetails which can include things like groupid, underlying
kafkaclusterId, standalone or distributed, etc. This will help expose any
cluster related information in the future.
Let me know if that would work?
Thanks,
Ma
Hi Magesh,
1. After ruminating for a little while on the inclusion of a method to
retrieve task configurations I've tentatively decided to remove it from the
proposal and place it in the rejected alternatives section. If anyone
presents a reasonable use case for it I'll be happy to discuss further
Chris,
I certainly would love to hear others thoughts on #1 but IMO, it might not
be as useful as ConnectorConfigs and as you mentioned, we could always add
it when the need arises.
Thanks for clarifying the details on my concern #2 regarding the
kafkaClusterId. While not a perfect fit in the inte
Hi Konstantine,
I didn't fully grasp the rationale behind adding default implementations
for the new interface at first; thanks for clarifying. I agree, this would
be good to include. I've updated the KIP with proposed defaults for the new
methods that simply throw UnsupportedOperationExceptions,
Thanks for the additions to the KIP Chris! The descriptions are quite clear
now.
With respect to 2) my main point was to say that you are targeting a
version that assumes Java 8, so adding the additional methods as default is
an option.
I think it's good practice to do that, even if we think it's
Hi Konstantine,
Thanks for your comments. I'll respond to them in the order you provided
them:
1. Clarify of ConnectClusterState code block:
Good point. I've altered the code block in the KIP to remove the outer
'interface ConnectClusterState {' snippet and include only the additional
methods. Ha
Resending an email I sent this Monday but didn't make it to the mailing
list:
Hi Chris,
thanks for the KIP!
I have the following initial comments:
1. In its current form, the code snippet gives the impression that this is
a new interface or an interface that completely replaces the existin
Hi Magesh,
Thanks for your comments. I'll address them in the order you provided them:
1 - Reason for exposing task configurations to REST extensions:
Yes, the motivation is a little thin for exposing task configs to REST
extensions. I can think of a few uses for this functionality, such as
attem
Hi Chris,
Thanks for the KIP. Overall, it looks good and straightforward to me.
I had a few questions on the new methods
1. I'm not sure if an extension would ever require the task configs. An
extension generally should only require the health and current state of the
connector which includes th
Hi all,
I've posted "KIP-454: Expansion of the ConnectClusterState interface",
which proposes that we add provide more information about the Connect
cluster to REST extensions.
The KIP can be found at
https://cwiki.apache.org/confluence/display/KAFKA/KIP-454%3A+Expansion+of+the+ConnectClusterStat
20 matches
Mail list logo
