1996fanrui commented on code in PR #24088: URL: https://github.com/apache/flink/pull/24088#discussion_r1454841243
########## flink-core/src/main/java/org/apache/flink/configuration/Configuration.java: ########## Review Comment: I try to mark it before, but the CI will be failed after marking. ## Reason: 1. All methods are `@Public`, if method without any annotation and current class is marked as `@Public`. 2. In the minor version, flink doesn't allow one method is changed from `@Public` to `@Internal`. I think it can be done in Flink-2.0 (FLINK-34082). ########## flink-core/src/test/java/org/apache/flink/configuration/ConfigurationTest.java: ########## @@ -554,6 +554,45 @@ void testMapParserErrorDoesNotLeakSensitiveData() { .doesNotContain("secret_value")); } + @TestTemplate + void testGetWithOverrideDefault() { + final Configuration conf = new Configuration(); Review Comment: Good catch, updated! ########## flink-core/src/main/java/org/apache/flink/configuration/Configuration.java: ########## @@ -179,7 +182,9 @@ public String getString(String key, String defaultValue) { * * @param configOption The configuration option * @return the (default) value associated with the given config option + * @deprecated use {@link #get(ConfigOption)} or {@link #getOptional(ConfigOption)} */ + @Deprecated Review Comment: Sorry, I don't know what difference between them? I added `@Deprecated` due to https://github.com/apache/flink/pull/23758. I see it added `@Deprecated` to `RestartStrategies` class directly. -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org