tillrohrmann commented on a change in pull request #10532:
[FLINK-15053][runtime] Escape all dynamical property values for taskmanager
URL: https://github.com/apache/flink/pull/10532#discussion_r360343306
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesResourceManager.java
##########
@@ -304,7 +304,7 @@ private void removePodIfTerminated(KubernetesPod pod) {
final String mainClassArgs = "--" +
CommandLineOptions.CONFIG_DIR_OPTION.getLongOpt() + " " +
flinkConfig.getString(KubernetesConfigOptions.FLINK_CONF_DIR) + " " +
- BootstrapTools.getDynamicProperties(flinkClientConfig,
flinkConfig);
+ String.join(" ",
BootstrapTools.getDynamicProperties(flinkClientConfig, flinkConfig));
Review comment:
I think we should not move the responsibility of generating the dynamic
properties string outside of `BootstrapTools` because it duplicates logic now.
Instead I would suggest to create another method in `BootstrapTools` which
returns a `Collection<String>/String[]` from a given `Configuration` and which
can be called by `BootstrapTools.getDynamicProperties`.
Maybe one could rename `getDynamicProperties` into
`getDynamicPropertiesAsString` or so.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
