Zoltan Chovan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23451 )

Change subject: KUDU-3662 [6/n] Add createTable flag and table initialization 
logic
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23451/1/java/kudu-replication/src/main/java/org/apache/kudu/replication/ReplicationTableInitializer.java
File 
java/kudu-replication/src/main/java/org/apache/kudu/replication/ReplicationTableInitializer.java:

http://gerrit.cloudera.org:8080/#/c/23451/1/java/kudu-replication/src/main/java/org/apache/kudu/replication/ReplicationTableInitializer.java@55
PS1, Line 55: sourceClient = new KuduClient.KuduClientBuilder(
            :             String. join(",", 
config.getSourceMasterAddresses())).build();
            :     sinkClient = new KuduClient.KuduClientBuilder(
            :             String.join(",", 
config.getSinkMasterAddresses())).build();
            :     sourceTableName = config.getTableName();
            :     sinkTableName = config.getSinkTableName();
            :     restoreOwner = config.getRestoreOwner();
Why not simply store these as a private final ReplicationJobConfig config class 
variable? In case the config class gets modified in the future we'll need to 
keep adding these copies here, which don't really serve any purpose, as the 
information is already accessible via the config object and there are no 
alterations/modifications are required here.


http://gerrit.cloudera.org:8080/#/c/23451/1/java/kudu-replication/src/main/java/org/apache/kudu/replication/ReplicationTableInitializer.java@78
PS1, Line 78: <p>This method is a Java port of the Scala method with the same 
name from
            :    * the kudu-backup module in 
KuduRestore.createTableRangePartitionByRangePartition
would it make sense to move this to a iseparate location so that both 
KuduRestore and the ReplicationInitializer could use it? Maybe 
kudu-backup-common or kudu-backup-tools?



--
To view, visit http://gerrit.cloudera.org:8080/23451
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1002b4ba272c1acaab351e3ff3f341ca327070d2
Gerrit-Change-Number: 23451
Gerrit-PatchSet: 1
Gerrit-Owner: Marton Greber <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Wed, 24 Sep 2025 07:03:45 +0000
Gerrit-HasComments: Yes

Reply via email to