zhuzhurk commented on code in PR #25234:
URL: https://github.com/apache/flink/pull/25234#discussion_r1741626266
##########
flink-core/src/main/java/org/apache/flink/configuration/GlobalConfiguration.java:
##########
@@ -45,6 +45,11 @@ public final class GlobalConfiguration {
private static final Logger LOG =
LoggerFactory.getLogger(GlobalConfiguration.class);
+ /**
+ * @deprecated This file is deprecated and is only used to help users
migrate their legacy
+ * configuration files to the new configuration file `config.yaml`.
+ */
+ @Deprecated @VisibleForTesting
Review Comment:
How about to introduce a separate `ConfigurationFileMigrationUtils` class?
It can be the only place to host logics which recognizes the legacy file.
##########
flink-core/src/main/java/org/apache/flink/configuration/GlobalConfiguration.java:
##########
@@ -143,31 +155,54 @@ public static Configuration loadConfiguration(
}
// get Flink yaml configuration file
- File yamlConfigFile = new File(confDirFile,
LEGACY_FLINK_CONF_FILENAME);
Configuration configuration;
-
- if (!yamlConfigFile.exists()) {
- yamlConfigFile = new File(confDirFile, FLINK_CONF_FILENAME);
+ if (loadLegacyConfigFile) {
+ File yamlConfigFile = new File(confDirFile,
LEGACY_FLINK_CONF_FILENAME);
Review Comment:
I prefer that the `GlobalConfiguration` code no longer see nor operate on
the legacy config file, just like mentioned in the comment above.
##########
flink-runtime/src/main/java/org/apache/flink/runtime/util/ConfigurationParserUtils.java:
##########
@@ -230,13 +228,9 @@ public static List<String>
migrateLegacyConfigurationToStandardYaml(
throw e;
}
- checkState(
Review Comment:
Why removing this check?
##########
flink-runtime/src/test/java/org/apache/flink/runtime/clusterframework/BootstrapToolsTest.java:
##########
@@ -298,8 +298,15 @@ private void
testWriteConfigurationAndReloadInternal(boolean standardYaml) throw
BootstrapTools.writeConfiguration(
flinkConfig, new File(flinkConfDir,
GlobalConfiguration.getFlinkConfFilename()));
- final Configuration loadedFlinkConfig =
-
GlobalConfiguration.loadConfiguration(flinkConfDir.getAbsolutePath());
+ final Configuration loadedFlinkConfig;
+ if (standardYaml) {
+ loadedFlinkConfig =
+
GlobalConfiguration.loadConfiguration(flinkConfDir.getAbsolutePath());
+ } else {
+ loadedFlinkConfig =
Review Comment:
We should not test cases that will not happen in production.
--
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]
