[
https://issues.apache.org/jira/browse/HUDI-2315?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17401193#comment-17401193
]
ASF GitHub Bot commented on HUDI-2315:
--------------------------------------
vinothchandar commented on a change in pull request #3491:
URL: https://github.com/apache/hudi/pull/3491#discussion_r690876029
##########
File path:
hudi-spark-datasource/hudi-spark2/src/main/java/org/apache/hudi/internal/DefaultSource.java
##########
@@ -61,11 +61,11 @@ public DataSourceReader createReader(DataSourceOptions
options) {
DataSourceOptions options) {
String instantTime =
options.get(DataSourceInternalWriterHelper.INSTANT_TIME_OPT_KEY).get();
String path = options.get("path").get();
- String tblName = options.get(HoodieWriteConfig.TABLE_NAME.key()).get();
+ String tblName =
options.get(HoodieWriteConfig.TABLE_NAME_VALUE.key()).get();
Review comment:
I'd like to avoid suffixes like _CFG or _CONF, given everything else
here is a config as well and the class is named xxxConfig.
I get your point though. Not a lot of great choices here. _TBL_NAME_ or just
_TABLE_?
##########
File path:
hudi-spark-datasource/hudi-spark2/src/main/java/org/apache/hudi/internal/DefaultSource.java
##########
@@ -61,11 +61,11 @@ public DataSourceReader createReader(DataSourceOptions
options) {
DataSourceOptions options) {
String instantTime =
options.get(DataSourceInternalWriterHelper.INSTANT_TIME_OPT_KEY).get();
String path = options.get("path").get();
- String tblName = options.get(HoodieWriteConfig.TABLE_NAME.key()).get();
+ String tblName =
options.get(HoodieWriteConfig.TABLE_NAME_VALUE.key()).get();
Review comment:
I'd like to avoid suffixes like _CFG or _CONF, given everything else
here is a config as well and the class is named xxxConfig. A filler word like
VALUE is rather innocuous comparatively
I get your point though. Not a lot of great choices here. _TBL_NAME_ or just
_TABLE_?
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieClusteringConfig.java
##########
@@ -90,33 +86,33 @@
.sinceVersion("0.9.0")
.withDocumentation("Number of partitions to skip from latest when
choosing partitions to create ClusteringPlan");
- public static final ConfigProperty<String> CLUSTERING_PLAN_SMALL_FILE_LIMIT
= ConfigProperty
+ public static final ConfigProperty<String>
CLUSTERING_STRATEGY_SMALL_FILE_LIMIT = ConfigProperty
.key(CLUSTERING_STRATEGY_PARAM_PREFIX + "small.file.limit")
.defaultValue(String.valueOf(600 * 1024 * 1024L))
.sinceVersion("0.7.0")
.withDocumentation("Files smaller than the size specified here are
candidates for clustering");
- public static final ConfigProperty<String> CLUSTERING_MAX_BYTES_PER_GROUP =
ConfigProperty
+ public static final ConfigProperty<String>
CLUSTERING_STRATEGY_MAX_BYTES_PER_OUTPUT_FILEGROUP = ConfigProperty
.key(CLUSTERING_STRATEGY_PARAM_PREFIX + "max.bytes.per.group")
.defaultValue(String.valueOf(2 * 1024 * 1024 * 1024L))
.sinceVersion("0.7.0")
.withDocumentation("Each clustering operation can create multiple output
file groups. Total amount of data processed by clustering operation"
+ " is defined by below two properties
(CLUSTERING_MAX_BYTES_PER_GROUP * CLUSTERING_MAX_NUM_GROUPS)."
+ " Max amount of data to be included in one group");
- public static final ConfigProperty<String> CLUSTERING_MAX_NUM_GROUPS =
ConfigProperty
+ public static final ConfigProperty<String> CLUSTERING_STRATEGY_MAX_GROUPS =
ConfigProperty
.key(CLUSTERING_STRATEGY_PARAM_PREFIX + "max.num.groups")
.defaultValue("30")
.sinceVersion("0.7.0")
.withDocumentation("Maximum number of groups to create as part of
ClusteringPlan. Increasing groups will increase parallelism");
- public static final ConfigProperty<String> CLUSTERING_TARGET_FILE_MAX_BYTES
= ConfigProperty
+ public static final ConfigProperty<String>
CLUSTERING_STRATEGY_TARGET_FILE_MAX_BYTES = ConfigProperty
.key(CLUSTERING_STRATEGY_PARAM_PREFIX + "target.file.max.bytes")
.defaultValue(String.valueOf(1024 * 1024 * 1024L))
.sinceVersion("0.7.0")
.withDocumentation("Each group can produce 'N'
(CLUSTERING_MAX_GROUP_SIZE/CLUSTERING_TARGET_FILE_SIZE) output file groups");
- public static final ConfigProperty<String> CLUSTERING_SORT_COLUMNS_PROPERTY
= ConfigProperty
+ public static final ConfigProperty<String> CLUSTERING_SORT_COLUMNS =
ConfigProperty
Review comment:
`CLUSTERING_` is redundant. made it consistent with the property prefix
. i.e `PLAN_STRATEGY_SORT_COLUMNS` and across the other places as well.
##########
File path:
hudi-spark-datasource/hudi-spark2/src/main/java/org/apache/hudi/internal/DefaultSource.java
##########
@@ -61,11 +61,11 @@ public DataSourceReader createReader(DataSourceOptions
options) {
DataSourceOptions options) {
String instantTime =
options.get(DataSourceInternalWriterHelper.INSTANT_TIME_OPT_KEY).get();
String path = options.get("path").get();
- String tblName = options.get(HoodieWriteConfig.TABLE_NAME.key()).get();
+ String tblName =
options.get(HoodieWriteConfig.TABLE_NAME_VALUE.key()).get();
Review comment:
Going with `TBL_NAME`
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieClusteringConfig.java
##########
@@ -90,33 +86,33 @@
.sinceVersion("0.9.0")
.withDocumentation("Number of partitions to skip from latest when
choosing partitions to create ClusteringPlan");
- public static final ConfigProperty<String> CLUSTERING_PLAN_SMALL_FILE_LIMIT
= ConfigProperty
+ public static final ConfigProperty<String>
CLUSTERING_STRATEGY_SMALL_FILE_LIMIT = ConfigProperty
.key(CLUSTERING_STRATEGY_PARAM_PREFIX + "small.file.limit")
.defaultValue(String.valueOf(600 * 1024 * 1024L))
.sinceVersion("0.7.0")
.withDocumentation("Files smaller than the size specified here are
candidates for clustering");
- public static final ConfigProperty<String> CLUSTERING_MAX_BYTES_PER_GROUP =
ConfigProperty
+ public static final ConfigProperty<String>
CLUSTERING_STRATEGY_MAX_BYTES_PER_OUTPUT_FILEGROUP = ConfigProperty
.key(CLUSTERING_STRATEGY_PARAM_PREFIX + "max.bytes.per.group")
.defaultValue(String.valueOf(2 * 1024 * 1024 * 1024L))
.sinceVersion("0.7.0")
.withDocumentation("Each clustering operation can create multiple output
file groups. Total amount of data processed by clustering operation"
+ " is defined by below two properties
(CLUSTERING_MAX_BYTES_PER_GROUP * CLUSTERING_MAX_NUM_GROUPS)."
+ " Max amount of data to be included in one group");
- public static final ConfigProperty<String> CLUSTERING_MAX_NUM_GROUPS =
ConfigProperty
+ public static final ConfigProperty<String> CLUSTERING_STRATEGY_MAX_GROUPS =
ConfigProperty
Review comment:
I don't think its file groups. this is loosely equivalent to a cluster
partition
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieBootstrapConfig.java
##########
@@ -49,22 +49,20 @@
.noDefaultValue()
.sinceVersion("0.6.0")
.withDocumentation("Base path of the dataset that needs to be
bootstrapped as a Hudi table");
- @Deprecated
- public static final String BOOTSTRAP_BASE_PATH_PROP =
BOOTSTRAP_BASE_PATH.key();
- public static final ConfigProperty<String> BOOTSTRAP_MODE_SELECTOR =
ConfigProperty
+ public static final ConfigProperty<String> BOOTSTRAP_MODE_SELECTOR_CLASS =
ConfigProperty
.key("hoodie.bootstrap.mode.selector")
.defaultValue(MetadataOnlyBootstrapModeSelector.class.getCanonicalName())
.sinceVersion("0.6.0")
.withDocumentation("Selects the mode in which each file/partition in the
bootstrapped dataset gets bootstrapped");
- public static final ConfigProperty<String> FULL_BOOTSTRAP_INPUT_PROVIDER =
ConfigProperty
+ public static final ConfigProperty<String>
FULL_BOOTSTRAP_INPUT_PROVIDER_CLASS = ConfigProperty
Review comment:
fixed .
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieBootstrapConfig.java
##########
@@ -107,8 +105,80 @@
.defaultValue(HFileBootstrapIndex.class.getName())
.sinceVersion("0.6.0")
Review comment:
done.
--
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]
> Maintain compatibility with pre Hudi 0.9.0 config keys, while marking them as
> deprecated
> ----------------------------------------------------------------------------------------
>
> Key: HUDI-2315
> URL: https://issues.apache.org/jira/browse/HUDI-2315
> Project: Apache Hudi
> Issue Type: Sub-task
> Reporter: Udit Mehrotra
> Assignee: Vinoth Chandar
> Priority: Blocker
> Labels: pull-request-available, release-blocker
> Fix For: 0.9.0
>
>
> With introduced a new configuration framework in HUDI-89, where all the
> String configurations have been replaced with ConfigProperty structure.
> However, we still want to maintain compatibility with existing pre Hudi 0.9.0
> config keys, so that it does not break existing users. At the same time, we
> will mark the previous config keys as deprecated so users can start migrating
> to the new configs.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
