C0urante commented on code in PR #12899:
URL: https://github.com/apache/kafka/pull/12899#discussion_r1034980195
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorCheckpointConnector.java:
##########
@@ -45,7 +45,7 @@ public class MirrorCheckpointConnector extends
SourceConnector {
private static final Logger log =
LoggerFactory.getLogger(MirrorCheckpointConnector.class);
private Scheduler scheduler;
- private MirrorConnectorConfig config;
+ private MirrorCheckpointConfig config;
Review Comment:
Should we update the class Javadoc as well?
```java
/** Replicate consumer group state between clusters. Emits checkpoint
records.
*
* @see MirrorCheckpointConfig for supported config properties.
*/
```
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorCheckpointConfig.java:
##########
@@ -0,0 +1,255 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.kafka.connect.mirror;
+
+import org.apache.kafka.common.config.ConfigDef;
+import org.apache.kafka.common.utils.ConfigUtils;
+
+import java.time.Duration;
+import java.util.List;
+import java.util.Map;
+
+public class MirrorCheckpointConfig extends MirrorConnectorConfig {
+
+ public static final String TOPIC_FILTER_CLASS = "topic.filter.class";
+ private static final String TOPIC_FILTER_CLASS_DOC = "TopicFilter to use.
Selects topics to replicate.";
+ public static final Class<?> TOPIC_FILTER_CLASS_DEFAULT =
DefaultTopicFilter.class;
Review Comment:
These and the `offset-syncs.topic.location`-related properties are defined
both here and in the `MirrorSourceConfig` class. To reduce duplication, do you
think it might make sense to put the constants for these properties (i.e., the
property names and their docstrings) in the `MirrorConnectorConfig` class,
along with the getter methods like `topicFilter` and
`offsetSyncsTopicLocation`? We could leave it up to subclasses to actually add
those properties to their respective `ConfigDef` objects so that they don't
show up in the docs for connectors that don't use them, but it would help
reduce the likelihood for typos and divergence in property names and docstrings.
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/MirrorSourceConnectorTest.java:
##########
@@ -333,4 +337,136 @@ public String upstreamTopic(String topic) {
new CustomReplicationPolicy(), new DefaultTopicFilter(), new
DefaultConfigPropertyFilter());
assertDoesNotThrow(() -> connector.isCycle(".b"));
}
+
+ @Test
+ public void testTaskConfigTopicPartitions() {
Review Comment:
Seems a little strange to move these tests to the
`MirrorSourceConnectorTest` suite since they don't actually test against
instances of a `MirrorSourceConnector` (and this might explain why @showuon had
trouble finding where these tests were moved to). Did you consider creating new
`MirrorSourceConfigTest` and `MirrorCheckpointConfigTest` classes?
##########
connect/mirror/src/main/java/org/apache/kafka/connect/mirror/MirrorSourceConnector.java:
##########
@@ -71,7 +71,7 @@ public class MirrorSourceConnector extends SourceConnector {
private static final AclBindingFilter ANY_TOPIC_ACL = new
AclBindingFilter(ANY_TOPIC, AccessControlEntryFilter.ANY);
private Scheduler scheduler;
- private MirrorConnectorConfig config;
+ private MirrorSourceConfig config;
Review Comment:
Same thought RE class Javadocs:
```java
* @see MirrorSourceConfig for supported config properties.
```
--
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
