wenjin272 commented on code in PR #888:
URL: https://github.com/apache/flink-agents/pull/888#discussion_r3568456211
##########
tools/install.sh:
##########
@@ -1563,6 +1564,16 @@ check_java() {
return 0
}
+# Upper bound (exclusive) on the Python minor version for the selected
+# Flink Agents release: 0.1.x / 0.2.x publish requires-python >=3.10,<3.12,
+# while 0.3.0+ publishes >=3.10,<3.13 (adds Python 3.12 support).
+python_minor_ceiling() {
+ case "$FLINK_AGENTS_VERSION" in
+ 0.1.*|0.2.*) printf '12' ;;
+ *) printf '13' ;;
+ esac
Review Comment:
Python 3.12 compatibility also depends on the selected Flink version, not
only the Flink Agents version. The installation docs require Flink 2.1+ for
Python 3.12, but `0.3.0 + Flink 1.20/2.0 + Python 3.12` currently passes this
validation and only fails later when `apache-flink==$FLINK_VERSION` is
installed.
Could we derive the Python constraint from both versions and revalidate it
whenever the Agents version, Flink version, or detected FLINK_HOME changes?
##########
tools/install.sh:
##########
@@ -1615,13 +1626,31 @@ resolve_python() {
die "No Python interpreter provided."
fi
if ! validate_python_bin "$input"; then
- die "Provided Python is invalid or unsupported (need >=3.10 and
<3.12): $input"
+ die "Provided Python is invalid or unsupported (need >=3.10 and
<3.$(python_minor_ceiling)): $input"
fi
PYTHON_BIN="$input"
ui_success "Using Python: $PYTHON_BIN"
return 0
}
+# The Python ceiling depends on the selected Flink Agents release (see
+# python_minor_ceiling), so editing the version at the confirm screen can
+# invalidate an interpreter that resolve_python already accepted — e.g.
+# Python 3.12 passes for 0.3.0 but pip install of 0.2.x requires <3.12.
+# Called after a version edit; re-resolves PYTHON_BIN when it no longer
+# satisfies the new release, mirroring the enable_pyflink edit arm.
+revalidate_python_for_agents_version() {
+ if [[ "$ENABLE_PYFLINK" != "Yes" ]] || [[ -z "${PYTHON_BIN:-}" ]]; then
+ return 0
+ fi
+ if validate_python_bin "$PYTHON_BIN"; then
+ return 0
+ fi
+ ui_warn "The selected Python (${PYTHON_BIN}) is incompatible with Flink
Agents ${FLINK_AGENTS_VERSION} (requires Python >=3.10 and
<3.$(python_minor_ceiling))"
+ PYTHON_BIN=""
Review Comment:
This revalidates `PYTHON_BIN`, but an existing `VENV_DIR` may still use the
incompatible interpreter. For example, after changing Agents from 0.3.0 to
0.2.1, this can select Python 3.11 while `setup_python_env` later reuses and
activates an existing Python 3.12 venv. Pip then still runs under Python 3.12
and `flink-agents==0.2.1` fails.
Could we also validate `$VENV_DIR/bin/python` and ask the user to select or
recreate the venv when it is incompatible? A regression test using an existing
Python 3.12 venv would cover this path.
--
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]
