You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/01/20 00:35:58 UTC

[GitHub] [flink-docker] zentol opened a new pull request #59: [FLINK-21037] Deduplicate configuration logic

zentol opened a new pull request #59:
URL: https://github.com/apache/flink-docker/pull/59


   Based on #58.
   
   One item that specifically needs a discussion is whether it is safe to also set the number of task slots for jobmanager processes. I would generally think so, as any user not setting it themselves may run into consistency issues anyway. (relevant: FLINK-21036)


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



[GitHub] [flink-docker] zentol merged pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
zentol merged pull request #59:
URL: https://github.com/apache/flink-docker/pull/59


   


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



[GitHub] [flink-docker] wangyang0918 commented on a change in pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #59:
URL: https://github.com/apache/flink-docker/pull/59#discussion_r560645542



##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       So we do not need to do the substitution after then. Why?




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



[GitHub] [flink-docker] zentol merged pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
zentol merged pull request #59:
URL: https://github.com/apache/flink-docker/pull/59


   


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



[GitHub] [flink-docker] zentol commented on a change in pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #59:
URL: https://github.com/apache/flink-docker/pull/59#discussion_r561774922



##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       I think you're being tricked by the diff; it is at the end of prepare_configuration.




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



[GitHub] [flink-docker] zentol commented on a change in pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #59:
URL: https://github.com/apache/flink-docker/pull/59#discussion_r561774922



##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       I think you're being tricked by the diff; it is at the end of prepare_configuration. ;)




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



[GitHub] [flink-docker] wangyang0918 commented on a change in pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #59:
URL: https://github.com/apache/flink-docker/pull/59#discussion_r561851637



##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       My bad. You are right.




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



[GitHub] [flink-docker] zentol commented on a change in pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
zentol commented on a change in pull request #59:
URL: https://github.com/apache/flink-docker/pull/59#discussion_r561774922



##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       I think you're being tricked by the diff; it is at the end of prepare_configuration.

##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       I think you're being tricked by the diff; it is at the end of prepare_configuration. ;)




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



[GitHub] [flink-docker] wangyang0918 commented on a change in pull request #59: [FLINK-21037] Deduplicate configuration logic

Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #59:
URL: https://github.com/apache/flink-docker/pull/59#discussion_r561851637



##########
File path: docker-entrypoint.sh
##########
@@ -112,41 +111,26 @@ if [ "$1" = "help" ]; then
 elif [ "$1" = "jobmanager" ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/jobmanager.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_STANDALONE} ]; then
     args=("${args[@]:1}")
 
-    prepare_job_manager_start
+    echo "Starting Job Manager"
 
     exec $(drop_privs_cmd) "$FLINK_HOME/bin/standalone-job.sh" start-foreground "${args[@]}"
 elif [ "$1" = ${COMMAND_HISTORY_SERVER} ]; then
     args=("${args[@]:1}")
 
     echo "Starting History Server"
 
-    if [ -n "${FLINK_PROPERTIES}" ]; then
-        echo "${FLINK_PROPERTIES}" >> "${CONF_FILE}"
-    fi
-    envsubst < "${CONF_FILE}" > "${CONF_FILE}.tmp" && mv "${CONF_FILE}.tmp" "${CONF_FILE}"

Review comment:
       My bad. You are right.




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