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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]