tillrohrmann commented on a change in pull request #14220:
URL: https://github.com/apache/flink/pull/14220#discussion_r531011602
##########
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:
I am not sure whether we are solving the problem correctly here. The
problem I have with this solution is that we are adjusting the formatting of
the `ClusterConfigHandler` because a user of this handler cannot parse the
values properly. This does not seem right to me. Moreover, I would find it
confusing to see a differently specified memory configuration in the web UI
where the config is displayed.
I could see two solutions to the problem: 1) Add the parsing logic to the
consumer of the data (namely the web UI). 2) Create an endpoint which serves
the JM memory configuration. In this handler, I'd be ok with outputting the
memory configuration as bytes.
----------------------------------------------------------------
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]
