[GitHub] [flink] wangyang0918 edited a comment on issue #10532: [FLINK-15053][runtime] Escape all dynamical property values for taskmanager

[GitBox <gi...@apache.org>]

wangyang0918 edited a comment on issue #10532: [FLINK-15053][runtime] Escape all dynamical property values for taskmanager
URL: https://github.com/apache/flink/pull/10532#issuecomment-570588137
 
 
   @tillrohrmann I take a serious consideration and tend to remove the windows escape code. Since the windows command does not have a very good compatibility. And in different version, we have to escape different characters. Although, using double quotes could satisfy almost the cases. `TaskManagerLoadingDynamicPropertiesITCase` will be a unstable test in the future.
   
   Go back to the changes in the PR, the `getDynamicPropertiesAsString` will only be used for Yarn and Kubernetes. Maybe the flink client is running in windows, the flink `YarnResourceManager`, `KubernetesResourceManager`, `YarnTaskExecutorRunner`, `KubernetesTaskExecutorRunner` are always running on unix-like OS. So the current implementation may go the wrong way and exceed what we want to test.
   
   Ignoring the `TaskManagerLoadingDynamicPropertiesITCase` test when running on windows is enough. 
   
   WDYT?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services