xushiyan commented on a change in pull request #4175:
URL: https://github.com/apache/hudi/pull/4175#discussion_r829735597
##########
File path:
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java
##########
@@ -240,41 +240,10 @@ public HiveSyncConfig(TypedProperties props) {
this.syncAsSparkDataSourceTable =
getBooleanOrDefault(HIVE_SYNC_AS_DATA_SOURCE_TABLE);
this.sparkSchemaLengthThreshold =
getIntOrDefault(HIVE_SYNC_SCHEMA_STRING_LENGTH_THRESHOLD);
this.createManagedTable = getBooleanOrDefault(HIVE_CREATE_MANAGED_TABLE);
- this.bucketSpec = props.getString(HIVE_SYNC_BUCKET_SYNC_SPEC.key(), null);
+ this.bucketSpec = getStringOrDefault(HIVE_SYNC_BUCKET_SYNC_SPEC);
Review comment:
@rmahindra123 Not sure why this was particularly set to `null`. seems
like the default `""` is ok.
##########
File path:
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/HiveSyncConfig.java
##########
@@ -240,41 +240,10 @@ public HiveSyncConfig(TypedProperties props) {
this.syncAsSparkDataSourceTable =
getBooleanOrDefault(HIVE_SYNC_AS_DATA_SOURCE_TABLE);
this.sparkSchemaLengthThreshold =
getIntOrDefault(HIVE_SYNC_SCHEMA_STRING_LENGTH_THRESHOLD);
this.createManagedTable = getBooleanOrDefault(HIVE_CREATE_MANAGED_TABLE);
- this.bucketSpec = props.getString(HIVE_SYNC_BUCKET_SYNC_SPEC.key(), null);
+ this.bucketSpec = getStringOrDefault(HIVE_SYNC_BUCKET_SYNC_SPEC);
this.syncComment = getBooleanOrDefault(HIVE_SYNC_COMMENT);
}
- // enhance the similar function in child class
- public static HiveSyncConfig copy(HiveSyncConfig cfg) {
Review comment:
this is not used.
##########
File path:
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/AbstractSyncTool.java
##########
@@ -41,7 +41,7 @@ public AbstractSyncTool(TypedProperties props, Configuration
conf, FileSystem fs
@Deprecated
public AbstractSyncTool(Properties props, FileSystem fileSystem) {
- this(new TypedProperties(props), new Configuration(), fileSystem);
+ this(new TypedProperties(props), fileSystem.getConf(), fileSystem);
Review comment:
@rmahindra123 if user provides an fs, shall we always make use of its
conf? Actually not sure why the new API allow passing a different conf rather
than use the one from `fileSystem`
##########
File path:
hudi-sync/hudi-hive-sync/src/main/java/org/apache/hudi/hive/replication/GlobalHiveSyncConfig.java
##########
@@ -18,15 +18,24 @@
package org.apache.hudi.hive.replication;
-import com.beust.jcommander.Parameter;
+import org.apache.hudi.common.config.TypedProperties;
import org.apache.hudi.hive.HiveSyncConfig;
+import com.beust.jcommander.Parameter;
+
public class GlobalHiveSyncConfig extends HiveSyncConfig {
@Parameter(names = {"--replicated-timestamp"}, description = "Add globally
replicated timestamp to enable consistent reads across clusters")
public String globallyReplicatedTimeStamp;
+ public GlobalHiveSyncConfig() {
+ }
+
+ public GlobalHiveSyncConfig(TypedProperties props) {
+ super(props);
+ }
+
public static GlobalHiveSyncConfig copy(GlobalHiveSyncConfig cfg) {
- GlobalHiveSyncConfig newConfig = new GlobalHiveSyncConfig();
+ GlobalHiveSyncConfig newConfig = new GlobalHiveSyncConfig(cfg.getProps());
newConfig.basePath = cfg.basePath;
newConfig.assumeDatePartitioning = cfg.assumeDatePartitioning;
Review comment:
the existing copy does not seem to copy everything, like `props`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
