C0urante commented on code in PR #13373:
URL: https://github.com/apache/kafka/pull/13373#discussion_r1149838213
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceConnector.java:
##########
@@ -540,10 +601,13 @@ Map<String, Config> describeTopicConfigs(Set<String>
topics)
Config targetConfig(Config sourceConfig) {
List<ConfigEntry> entries = sourceConfig.entries().stream()
- .filter(x -> !x.isDefault() && !x.isReadOnly() && !x.isSensitive())
- .filter(x -> x.source() !=
ConfigEntry.ConfigSource.STATIC_BROKER_CONFIG)
- .filter(x -> shouldReplicateTopicConfigurationProperty(x.name()))
- .collect(Collectors.toList());
+ .filter(x ->
(!useIncrementalAlterConfigs.equals(MirrorSourceConfig.NEVER_USE_INCREMENTAL_ALTER_CONFIG)
+ || (x.isDefault() &&
shouldReplicateSourceDefault(x.name())))
+ || (!x.isDefault() &&
useIncrementalAlterConfigs.equals(MirrorSourceConfig.NEVER_USE_INCREMENTAL_ALTER_CONFIG)))
+ .filter(x -> !x.isReadOnly() && !x.isSensitive())
+ .filter(x -> x.source() !=
ConfigEntry.ConfigSource.STATIC_BROKER_CONFIG)
+ .filter(x ->
shouldReplicateTopicConfigurationProperty(x.name()))
Review Comment:
Hmm... I was imagining something slightly different:
For the `ConfigPropertyFilter` interface, the same property would never be
tested against both methods (`shouldReplicateSourceDefault` and
`shouldReplicateConfigProperty`). This lines up with the language in the KIP,
which is that users "will be able to configure this method to return true in
order to include source cluster's default configurations in the replication"
(it mentions nothing about also having to return true from
`shouldReplicateConfigProperty`). It also avoids some redundancy (you shouldn't
have to change multiple methods to accommodate a single property).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
