xushiyan commented on code in PR #7722:
URL: https://github.com/apache/hudi/pull/7722#discussion_r1086228641


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieInstantTimeGenerator.java:
##########
@@ -44,7 +44,7 @@ public class HoodieInstantTimeGenerator {
   public static final int MILLIS_INSTANT_TIMESTAMP_FORMAT_LENGTH = 
MILLIS_INSTANT_TIMESTAMP_FORMAT.length();
   // Formatter to generate Instant timestamps
   // Unfortunately millisecond format is not parsable as is 
https://bugs.openjdk.java.net/browse/JDK-8031085. hence have to do appendValue()
-  private static DateTimeFormatter MILLIS_INSTANT_TIME_FORMATTER = new 
DateTimeFormatterBuilder().appendPattern(SECS_INSTANT_TIMESTAMP_FORMAT)
+  public static DateTimeFormatter MILLIS_INSTANT_TIME_FORMATTER = new 
DateTimeFormatterBuilder().appendPattern(SECS_INSTANT_TIMESTAMP_FORMAT)

Review Comment:
   this should be reverted



##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -752,17 +749,23 @@ public Builder 
setLayoutVersion(Option<TimelineLayoutVersion> layoutVersion) {
       return this;
     }
 
-    public Builder setProperties(Properties properties) {
-      this.props = properties;
+    public Builder setMetaserverConfig(Properties props) {
+      this.metaserverConfig = new 
HoodieMetaserverConfig.Builder().fromProperties(props).build();

Review Comment:
   you just need a new method to take in metaserver config, why removing 
existing API `setProperties()` ?



##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableMetaClient.java:
##########
@@ -680,15 +680,12 @@ public void initializeBootstrapDirsIfNotExists() throws 
IOException {
 
   private static HoodieTableMetaClient newMetaClient(Configuration conf, 
String basePath, boolean loadActiveTimelineOnLoad,
       ConsistencyGuardConfig consistencyGuardConfig, 
Option<TimelineLayoutVersion> layoutVersion,
-      String payloadClassName, String recordMergerStrategy, 
FileSystemRetryConfig fileSystemRetryConfig, Properties props) {
-    HoodieMetaserverConfig metaserverConfig = null == props
-        ? new HoodieMetaserverConfig.Builder().build()
-        : new HoodieMetaserverConfig.Builder().fromProperties(props).build();
+      String payloadClassName, String recordMergerStrategy, 
FileSystemRetryConfig fileSystemRetryConfig, HoodieMetaserverConfig 
metaserverConfig) {
     return metaserverConfig.isMetaserverEnabled()
         ? (HoodieTableMetaClient) 
ReflectionUtils.loadClass("org.apache.hudi.common.table.HoodieTableMetaserverClient",
-        new Class<?>[] {Configuration.class, ConsistencyGuardConfig.class, 
String.class, FileSystemRetryConfig.class, String.class, String.class, 
HoodieMetaserverConfig.class},
-        conf, consistencyGuardConfig, recordMergerStrategy, 
fileSystemRetryConfig,
-        props.getProperty(HoodieTableConfig.DATABASE_NAME.key()), 
props.getProperty(HoodieTableConfig.NAME.key()), metaserverConfig)
+        new Class<?>[] {Configuration.class, String.class, 
ConsistencyGuardConfig.class, String.class, FileSystemRetryConfig.class,
+            String.class, String.class, HoodieMetaserverConfig.class}, conf, 
basePath, consistencyGuardConfig, recordMergerStrategy, fileSystemRetryConfig,
+        metaserverConfig.getDatabaseName(), metaserverConfig.getTableName(), 
metaserverConfig)

Review Comment:
   you already passed `HoodieMetaserverConfig`, then no need to pass database 
name and table name separately. pls minimize number of args



##########
packaging/bundle-validation/validate.sh:
##########
@@ -185,6 +186,42 @@ test_kafka_connect_bundle() {
     kill $ZOOKEEPER_PID $KAFKA_SERVER_PID $SCHEMA_REG_PID
 }
 
+##
+# Function to test the hudi metaserver bundles.
+#
+# env vars (defined in container):
+#   SPARK_HOME: path to the spark directory
+##
+test_hudi_metaserver_bundles () {
+    echo "::warning::validate.sh setting up hudi metaserver bundles validation"
+
+    echo "::warning::validate.sh Start hudi metaserver"
+    java -jar $JARS_DIR/metaserver.jar & local METASEVER=$!
+
+    echo "::warning::validate.sh Start hive server"
+    $DERBY_HOME/bin/startNetworkServer -h 0.0.0.0 &
+    local DERBY_PID=$!
+    $HIVE_HOME/bin/hiveserver2 --hiveconf 
hive.aux.jars.path=$JARS_DIR/hadoop-mr.jar &
+    local HIVE_PID=$!

Review Comment:
   @minihippo pls file jira so we don't forget



-- 
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]

Reply via email to