HuangXingBo commented on code in PR #21770:
URL: https://github.com/apache/flink/pull/21770#discussion_r1116691854
##########
docs/layouts/shortcodes/generated/execution_config_configuration.html:
##########
@@ -15,7 +15,7 @@
<td>The max number of async i/o operation that the async lookup
join can trigger.</td>
</tr>
<tr>
- <td><h5>table.exec.async-lookup.output-mode</h5><br> <span
class="label label-primary">Batch</span> <span class="label label
-primary">Streaming</span></td>
+ <td><h5>table.exec.async-lookup.output-mode</h5><br> <span
class="label label-primary">Batch</span> <span class="label
label-primary">Streaming</span></td>
Review Comment:
unrelated change?
##########
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:
I think we can split these unrelated change into different commit.
##########
flink-python/src/main/java/org/apache/flink/python/env/PythonDependencyInfo.java:
##########
@@ -100,13 +104,32 @@ public PythonDependencyInfo(
@Nullable String requirementsCacheDir,
@Nonnull Map<String, String> archives,
@Nonnull String pythonExec,
- @Nonnull String executionMode) {
+ @Nullable String pythonPath) {
+ this(
+ pythonFiles,
+ requirementsFilePath,
+ requirementsCacheDir,
+ archives,
+ pythonExec,
+ PYTHON_EXECUTION_MODE.defaultValue(),
+ pythonPath);
+ }
Review Comment:
For compatibility reasons, we should not modify this constructor
##########
flink-python/src/main/java/org/apache/flink/python/env/PythonDependencyInfo.java:
##########
@@ -133,6 +156,10 @@ public String getExecutionMode() {
return executionMode;
}
+ public String getPythonPath() {
Review Comment:
```suggestion
public Optional<String> getPythonPath() {
```
##########
flink-python/src/main/java/org/apache/flink/python/PythonOptions.java:
##########
@@ -122,6 +122,19 @@ public class PythonOptions {
+ "optional parameter exists. The option
is equivalent to the command line option "
+ "\"-pyreq\".");
+ /** The configuration allows user to define python path for client and
workers. */
+ public static final ConfigOption<String> PYTHON_PATH =
+ ConfigOptions.key("env.PYTHONPATH")
+ .stringType()
+ .noDefaultValue()
+ .withDescription(
+ Description.builder()
+ .text(
+ "Specify the path on the Worker
Node where the Flink Python Dependencies are installed, which "
+ + "gets added into the
PYTHONPATH of the Python Worker. "
+ + "The option is
equivalent to the command line option \"-penv.PYTHONPATH\".")
Review Comment:
Do you mean that you will add a new command line option `-penv.PYTHONPATH`?
##########
flink-python/src/main/java/org/apache/flink/python/env/AbstractPythonEnvironmentManager.java:
##########
@@ -166,6 +166,14 @@ public Map<String, String>
constructEnvironmentVariables(String baseDirectory)
constructFilesDirectory(env, baseDirectory);
+ if (this.dependencyInfo.getPythonPath() != null) {
+ List<String> configuredPythonPath = new ArrayList<>();
+ configuredPythonPath.add(this.dependencyInfo.getPythonPath());
+ appendToPythonPath(env, configuredPythonPath);
+ }
Review Comment:
```suggestion
if (dependencyInfo.getPythonPath().isPresent()) {
appendToPythonPath(env,
Collections.singletonList(dependencyInfo.getPythonPath().get()));
}
```
##########
flink-python/src/main/java/org/apache/flink/python/PythonOptions.java:
##########
@@ -122,6 +122,19 @@ public class PythonOptions {
+ "optional parameter exists. The option
is equivalent to the command line option "
+ "\"-pyreq\".");
+ /** The configuration allows user to define python path for client and
workers. */
+ public static final ConfigOption<String> PYTHON_PATH =
+ ConfigOptions.key("env.PYTHONPATH")
Review Comment:
`env.PYTHONPATH` is a bit odd as an option name. We might change the name to
something like `python.pythonpath`, consistent with other python options
##########
flink-python/src/main/java/org/apache/flink/python/env/PythonDependencyInfo.java:
##########
@@ -133,6 +156,10 @@ public String getExecutionMode() {
return executionMode;
}
+ public String getPythonPath() {
+ return pythonPath;
Review Comment:
```suggestion
return Optional.ofNullable(pythonPath);
```
--
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]
