tillrohrmann commented on a change in pull request #13747:
URL: https://github.com/apache/flink/pull/13747#discussion_r510950775
##########
File path: flink-dist/src/main/flink-bin/bin/jobmanager.sh
##########
@@ -51,6 +51,10 @@ if [[ $STARTSTOP == "start" ]] || [[ $STARTSTOP ==
"start-foreground" ]]; then
args+=("--webui-port")
args+=("${WEBUIPORT}")
fi
+
+ if [ ! -z "${DYNAMIC_PARAMETERS+x}" ]; then
Review comment:
Why is a `+x` appended to `DYNAMIC_PARAMETERS`? In the other scripts
this suffix is missing.
##########
File path: flink-dist/src/main/flink-bin/bin/jobmanager.sh
##########
@@ -51,6 +51,10 @@ if [[ $STARTSTOP == "start" ]] || [[ $STARTSTOP ==
"start-foreground" ]]; then
args+=("--webui-port")
args+=("${WEBUIPORT}")
fi
+
+ if [ ! -z "${DYNAMIC_PARAMETERS+x}" ]; then
+ for v in ${DYNAMIC_PARAMETERS[@]}; do args+=($v); done
Review comment:
Why do we have to add it in a loop to `args`? Is this some bash magic?
##########
File path:
flink-dist/src/test/java/org/apache/flink/dist/BashJavaUtilsITCase.java
##########
@@ -72,20 +79,23 @@ public void
testGetTmResourceParamsConfigsWithDynamicProperties() throws Excepti
@Test
public void testGetJmResourceParams() throws Exception {
- int expectedResultLines = 1;
+ int expectedResultLines = 2;
Review comment:
I guess that this commit should be merged together with the previous one
because w/o, the previous commit should fail in this test here.
----------------------------------------------------------------
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]
