HuangXingBo commented on code in PR #21770:
URL: https://github.com/apache/flink/pull/21770#discussion_r1113953192


##########
flink-python/src/main/java/org/apache/flink/client/python/PythonEnvUtils.java:
##########
@@ -324,18 +325,25 @@ private static void addToPythonPath(PythonEnvironment 
env, List<Path> pythonFile
     /**
      * Starts python process.
      *
+     * @param config configs.
      * @param pythonEnv the python Environment which will be in a process.
      * @param commands the commands that python process will execute.
      * @return the process represent the python process.
      * @throws IOException Thrown if an error occurred when python process 
start.
      */
     static Process startPythonProcess(
-            PythonEnvironment pythonEnv, List<String> commands, boolean 
redirectToPipe)
+            ReadableConfig config,
+            PythonEnvironment pythonEnv,
+            List<String> commands,
+            boolean redirectToPipe)
             throws IOException {
         ProcessBuilder pythonProcessBuilder = new ProcessBuilder();
         Map<String, String> env = pythonProcessBuilder.environment();
+
         if (pythonEnv.pythonPath != null) {
-            String defaultPythonPath = env.get("PYTHONPATH");
+            String defaultPythonPath =
+                    
config.getOptional(PYTHON_PATH).orElse(env.get("PYTHON_PATH"));

Review Comment:
   ```suggestion
                       
config.getOptional(PYTHON_PATH).orElse(env.get("PYTHONPATH"));
   ```



##########
docs/static/generated/rest_v1_dispatcher.yml:
##########
@@ -6,7 +6,7 @@ info:
   license:
     name: Apache 2.0
     url: https://www.apache.org/licenses/LICENSE-2.0.html
-  version: v1/1.17-SNAPSHOT
+  version: v1/1.18-SNAPSHOT

Review Comment:
   Modifications not related to this feature?



##########
flink-python/src/main/java/org/apache/flink/client/python/PythonEnvUtils.java:
##########
@@ -324,18 +325,25 @@ private static void addToPythonPath(PythonEnvironment 
env, List<Path> pythonFile
     /**
      * Starts python process.
      *
+     * @param config configs.
      * @param pythonEnv the python Environment which will be in a process.
      * @param commands the commands that python process will execute.
      * @return the process represent the python process.
      * @throws IOException Thrown if an error occurred when python process 
start.
      */
     static Process startPythonProcess(
-            PythonEnvironment pythonEnv, List<String> commands, boolean 
redirectToPipe)
+            ReadableConfig config,
+            PythonEnvironment pythonEnv,
+            List<String> commands,
+            boolean redirectToPipe)
             throws IOException {
         ProcessBuilder pythonProcessBuilder = new ProcessBuilder();
         Map<String, String> env = pythonProcessBuilder.environment();
+
         if (pythonEnv.pythonPath != null) {
-            String defaultPythonPath = env.get("PYTHONPATH");
+            String defaultPythonPath =
+                    
config.getOptional(PYTHON_PATH).orElse(env.get("PYTHON_PATH"));

Review Comment:
   And I prefer these logic is in `preparePythonEnvironment` rather than adding 
another param `ReadableConfig` to `startPythonProcess`



##########
flink-python/src/main/java/org/apache/flink/client/python/PythonEnvUtils.java:
##########
@@ -324,18 +325,25 @@ private static void addToPythonPath(PythonEnvironment 
env, List<Path> pythonFile
     /**
      * Starts python process.
      *
+     * @param config configs.
      * @param pythonEnv the python Environment which will be in a process.
      * @param commands the commands that python process will execute.
      * @return the process represent the python process.
      * @throws IOException Thrown if an error occurred when python process 
start.
      */
     static Process startPythonProcess(
-            PythonEnvironment pythonEnv, List<String> commands, boolean 
redirectToPipe)
+            ReadableConfig config,
+            PythonEnvironment pythonEnv,
+            List<String> commands,
+            boolean redirectToPipe)
             throws IOException {
         ProcessBuilder pythonProcessBuilder = new ProcessBuilder();
         Map<String, String> env = pythonProcessBuilder.environment();
+
         if (pythonEnv.pythonPath != null) {
-            String defaultPythonPath = env.get("PYTHONPATH");
+            String defaultPythonPath =
+                    
config.getOptional(PYTHON_PATH).orElse(env.get("PYTHON_PATH"));

Review Comment:
   Will the `PYTHON_PATH` configuration work on both the client and the cluster 
side?
   



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

To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to