XComp commented on a change in pull request #14220:
URL: https://github.com/apache/flink/pull/14220#discussion_r531459331
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/rest/messages/ClusterConfigurationInfo.java
##########
@@ -45,10 +59,19 @@ public static ClusterConfigurationInfo from(Configuration
config) {
final Map<String, String>
configurationWithHiddenSensitiveValues =
ConfigurationUtils.hideSensitiveValues(config.toMap());
for (Map.Entry<String, String> keyValuePair :
configurationWithHiddenSensitiveValues.entrySet()) {
- clusterConfig.add(new
ClusterConfigurationInfoEntry(keyValuePair.getKey(), keyValuePair.getValue()));
+
clusterConfig.add(createClusterConfigurationInfoEntry(keyValuePair.getKey(),
keyValuePair.getValue()));
}
return clusterConfig;
}
+ @VisibleForTesting
+ static ClusterConfigurationInfoEntry<?>
createClusterConfigurationInfoEntry(String key, String valueStr) {
+ if (MEMORY_OPTION_KEYS.contains(key)) {
+ return new ClusterConfigurationInfoEntry<>(key,
MemorySize.parse(valueStr).getBytes());
+ } else {
+ return new ClusterConfigurationInfoEntry<>(key,
valueStr);
+ }
Review comment:
And another clarification here: We're adjusting the memory configuration
here already since we're transforming any memory setting from whatever unit the
user used into bytes.
Anyway, it's good that you raised your concerns: I got back to @vthinkxie
re-iterating over the problem ones more realizing that I misunderstood him.
He's fine with doing the conversion on the client side (i.e. in the web UI).
Hence, we're gonna go with your solution (1).
----------------------------------------------------------------
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]
