Airblader commented on a change in pull request #16150:
URL: https://github.com/apache/flink/pull/16150#discussion_r650171733
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -661,14 +665,16 @@ public boolean containsKey(String key) {
*/
@PublicEvolving
public boolean contains(ConfigOption<?> configOption) {
+ final boolean canBePrefixMap = canBePrefixMap(configOption);
+
+ // first try the current key
synchronized (this.confData) {
- // first try the current key
- if (this.confData.containsKey(configOption.key())) {
+ if (containsInternal(configOption.key(), canBePrefixMap)) {
return true;
} else if (configOption.hasFallbackKeys()) {
// try the fallback keys
for (FallbackKey fallbackKey : configOption.fallbackKeys()) {
- if (this.confData.containsKey(fallbackKey.getKey())) {
+ if (containsInternal(configOption.key(), canBePrefixMap)) {
Review comment:
Shouldn't this be fallbackKey?
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -759,31 +791,49 @@ public boolean contains(ConfigOption<?> configOption) {
}
synchronized (this.confData) {
+ if (canBePrefixMap) {
+ removePrefixMap(this.confData, key);
+ }
this.confData.put(key, value);
}
}
private Optional<Object> getRawValue(String key) {
+ return getRawValue(key, false);
+ }
+
+ private Optional<Object> getRawValue(String key, boolean canBePrefixMap) {
if (key == null) {
throw new NullPointerException("Key must not be null.");
}
synchronized (this.confData) {
- return Optional.ofNullable(this.confData.get(key));
+ final Object valueFromExactKey = this.confData.get(key);
+ if (!canBePrefixMap || valueFromExactKey != null) {
+ return Optional.ofNullable(valueFromExactKey);
+ }
+ final Map<String, String> valueFromPrefixMap =
+ convertToPropertiesPrefixed(confData, key);
+ if (valueFromPrefixMap.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(valueFromPrefixMap);
Review comment:
(This is obviously highly opinionated, but in this specific case I
prefer the current version better)
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -759,31 +791,49 @@ public boolean contains(ConfigOption<?> configOption) {
}
synchronized (this.confData) {
+ if (canBePrefixMap) {
+ removePrefixMap(this.confData, key);
+ }
this.confData.put(key, value);
}
}
private Optional<Object> getRawValue(String key) {
+ return getRawValue(key, false);
+ }
+
+ private Optional<Object> getRawValue(String key, boolean canBePrefixMap) {
if (key == null) {
throw new NullPointerException("Key must not be null.");
}
synchronized (this.confData) {
- return Optional.ofNullable(this.confData.get(key));
+ final Object valueFromExactKey = this.confData.get(key);
+ if (!canBePrefixMap || valueFromExactKey != null) {
+ return Optional.ofNullable(valueFromExactKey);
+ }
+ final Map<String, String> valueFromPrefixMap =
+ convertToPropertiesPrefixed(confData, key);
+ if (valueFromPrefixMap.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(valueFromPrefixMap);
Review comment:
(This is obviously highly opinionated, but in this specific case I
prefer the current version more)
##########
File path:
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -759,31 +791,49 @@ public boolean contains(ConfigOption<?> configOption) {
}
synchronized (this.confData) {
+ if (canBePrefixMap) {
+ removePrefixMap(this.confData, key);
+ }
this.confData.put(key, value);
}
}
private Optional<Object> getRawValue(String key) {
+ return getRawValue(key, false);
+ }
+
+ private Optional<Object> getRawValue(String key, boolean canBePrefixMap) {
if (key == null) {
throw new NullPointerException("Key must not be null.");
}
synchronized (this.confData) {
- return Optional.ofNullable(this.confData.get(key));
+ final Object valueFromExactKey = this.confData.get(key);
+ if (!canBePrefixMap || valueFromExactKey != null) {
+ return Optional.ofNullable(valueFromExactKey);
+ }
+ final Map<String, String> valueFromPrefixMap =
+ convertToPropertiesPrefixed(confData, key);
+ if (valueFromPrefixMap.isEmpty()) {
+ return Optional.empty();
+ }
+ return Optional.of(valueFromPrefixMap);
Review comment:
(This is obviously highly opinionated, but in this specific case I
prefer the current version)
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
