Extract pluggable interface for kafka cluster supplier#16272
Extract pluggable interface for kafka cluster supplier#16272beinan merged 1 commit intoprestodb:masterfrom
Conversation
yangy0000
changed the title
yangy0000
force-pushed
the
cluster-metadata
branch
2 times, most recently
from
1a9d37e to
3f1b0d5
Compare
yangy0000
force-pushed
the
cluster-metadata
branch
from
3f1b0d5 to
3887f2c
Compare
yangy0000
changed the title
|
@beinan can you help to review the PR , thanks |
beinan
left a comment
beinan
left a comment
There was a problem hiding this comment.
Looks good to me, just a couple of clarification questions.
|
|
||
| @Config("kafka.hide-internal-columns") | ||
| public KafkaConnectorConfig setHideInternalColumns(boolean hideInternalColumns) | ||
| @Config("kafka.cluster-metadata-supplier") |
There was a problem hiding this comment.
Shall we document this new configuration? Thanks!
There was a problem hiding this comment.
as for now, the only option for this configuration is the default option. do you recommend adding the document now or later when the new supplier is added?
There was a problem hiding this comment.
I see, we can document it later I think. I remember you also made some other pluggable interface before. Maybe you can document these new configuration together later.
| public List<HostAddress> getNodes() | ||
| { | ||
| return nodes; | ||
| } | ||
|
|
||
| @Config("kafka.nodes") | ||
| public KafkaConnectorConfig setNodes(String nodes) | ||
| { | ||
| this.nodes = (nodes == null) ? null : parseNodes(nodes).asList(); | ||
| return this; | ||
| } | ||
|
|
There was a problem hiding this comment.
Just wanna double confirm if this would break the existing config. is there any config migration required for this change?
There was a problem hiding this comment.
The existing config can work as-is, no config migration is needed.
The reason is default cluster metadata supplier(FileKafkaClusterMetadataSupplier) will load FileKafkaClusterMetadataSupplierConfig which is the copy of the config that has been removed from this file.
|
@yangy0000 / @beinan |
3 participants
The intention of this PR is to build the ground layer for Kafka multiple clusters support proposed in #15845
This PR built a pluggable Kafka cluster metadata supplier interface and moved the existing implementation into FileKafkaClusterMetadataSupplier.
Tested and verified at local.