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]