[GitHub] [flink-docker] zentol commented on a change in pull request #15: [FLINK-17346] De-duplicate process setup

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

zentol commented on a change in pull request #15:
URL: https://github.com/apache/flink-docker/pull/15#discussion_r414395476



##########
File path: testing/testing_lib.sh
##########
@@ -45,51 +45,42 @@ function build_image() {
     docker build -t "$image_name" "$dockerfile_dir"
 }
 
-function run_jobmanager() {
-    local dockerfile
-    dockerfile="$1"
+function internal_run() {
+    local dockerfile="$1"
+    local docker_run_command="$2"
+    local args="$3"
 
     local image_tag image_name
     image_tag="$(image_tag "$dockerfile")"
     image_name="$(image_name "$image_tag")"
 
-    echo >&2 "===> Starting ${image_tag} jobmanager..."
-
-    # Prints container ID
-    docker run \
-        --rm \
-        --detach \
-        --name "jobmanager" \
-        --network "$NETWORK_NAME" \
-        --publish 6123:6123 \
-        --publish 8081:8081 \
-        -e JOB_MANAGER_RPC_ADDRESS="jobmanager" \
-        "$image_name" \
-        jobmanager
+    echo >&2 "===> Starting ${image_tag} ${args}..."
+
+    eval "docker run --rm --detach --network $NETWORK_NAME -e JOB_MANAGER_RPC_ADDRESS=jobmanager ${docker_run_command} $image_name ${args}"
+}
+
+function internal_run_jobmanager() {
+    internal_run "$1" "--name jobmanager --publish 6123:6123 --publish 8081:8081 $2" jobmanager
+}
+
+function run_jobmanager() {
+    internal_run_jobmanager "$1" ""

Review comment:
       probably not, but I think it is better to be explicit about it. If another argument is added you're less likely to pass the new argument as the second one.




----------------------------------------------------------------
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