This is an automated email from the ASF dual-hosted git repository.
wenjin272 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/flink-agents.git
The following commit(s) were added to refs/heads/main by this push:
new 6f020c50 [tools] Keep install plan edits consistent (#899)
6f020c50 is described below
commit 6f020c5061bc1b444fbdb832d57ee7430517a62a
Author: Wenjin Xie <[email protected]>
AuthorDate: Tue Jul 14 17:48:32 2026 +0800
[tools] Keep install plan edits consistent (#899)
Co-authored-by: Codex <[email protected]>
---
tools/install.sh | 87 ++++++++++++++++-------
tools/test/helpers/load.bash | 2 +-
tools/test/unit/edit_plan_quote.bats | 3 +
tools/test/unit/reconcile_plan_after_edit.bats | 55 ++++++++++++++
tools/test/unit/revalidate_python_constraint.bats | 73 ++++++++++++++++++-
5 files changed, 190 insertions(+), 30 deletions(-)
diff --git a/tools/install.sh b/tools/install.sh
index 70fc516f..68963ca3 100755
--- a/tools/install.sh
+++ b/tools/install.sh
@@ -475,9 +475,10 @@ VERBOSE="${FLINK_AGENTS_VERBOSE:-0}"
DRY_RUN="${FLINK_AGENTS_DRY_RUN:-0}"
HELP=0
PYFLINK_ACTUALLY_ENABLED=0
-# Set to 1 when a version edit leaves an existing venv on an incompatible
-# interpreter and the user opts to recreate it; setup_python_env honors it.
-RECREATE_VENV=0
+# Exact venv path the user approved for recreation after an edit made its
+# interpreter incompatible. Keeping the path, rather than a boolean, prevents
+# a later VENV_DIR edit from transferring that destructive intent elsewhere.
+RECREATE_VENV_PATH=""
print_usage() {
cat <<EOF
@@ -992,6 +993,29 @@ edit_prompt_flink_home() {
export FLINK_HOME
}
+# Recompute derived values and constraints once after every successful edit.
+# Flink detection is limited to Flink-related actions because invoking
+# bin/flink may start a JVM; Python and venv validation is cheap and applies
+# after every action so path-bound recreation state cannot become stale.
+reconcile_plan_after_edit() {
+ local action="$1"
+ case "$action" in
+ install_flink|flink_version|install_dir|flink_home)
+ if [[ "$INSTALL_FLINK" == "Yes" ]]; then
+ FLINK_HOME="${INSTALL_DIR}/flink-${FLINK_VERSION}"
+ else
+ if detect_flink_version_from_home; then
+ ui_success "Detected Flink version: $FLINK_VERSION"
+ else
+ ui_warn "Could not auto-detect Flink version; keeping
${FLINK_VERSION}"
+ fi
+ fi
+ FLINK_MAJOR_MINOR="$(flink_major_minor "$FLINK_VERSION")"
+ ;;
+ esac
+ revalidate_python_constraint
+}
+
# Show a menu of currently-applicable plan fields and re-prompt for the
# selected one. After the edit, recomputes FLINK_HOME / FLINK_MAJOR_MINOR
# so the next show_install_plan reflects the change.
@@ -1018,7 +1042,7 @@ edit_plan_dump_state() {
printf 'VENV_DIR=%s\n' "$(edit_plan_quote
"$VENV_DIR")"
printf 'PYTHON_BIN=%s\n' "$(edit_plan_quote
"${PYTHON_BIN:-}")"
printf 'FLINK_AGENTS_VERSION=%s\n' "$(edit_plan_quote
"$FLINK_AGENTS_VERSION")"
- printf 'RECREATE_VENV=%s\n' "$(edit_plan_quote
"${RECREATE_VENV:-0}")"
+ printf 'RECREATE_VENV_PATH=%s\n' "$(edit_plan_quote
"${RECREATE_VENV_PATH:-}")"
} > "$out"
}
@@ -1115,7 +1139,6 @@ edit_plan_interactive() {
case "$action" in
flink_agents_version)
prompt_flink_agents_version_interactive || true
- revalidate_python_constraint
;;
install_flink)
if choose_install_method_interactive "Install Flink?"; then
@@ -1131,20 +1154,13 @@ edit_plan_interactive() {
"/path/to/flink-install-dir")"
INSTALL_DIR="$(normalize_path "$INSTALL_DIR")"
fi
- FLINK_HOME="${INSTALL_DIR}/flink-${FLINK_VERSION}"
else
INSTALL_FLINK=No
edit_prompt_flink_home
- detect_flink_version_from_home || ui_warn "Could not
auto-detect Flink version; keeping ${FLINK_VERSION}"
fi
- FLINK_MAJOR_MINOR="$(flink_major_minor "$FLINK_VERSION")"
- revalidate_python_constraint
;;
flink_version)
prompt_flink_version_interactive || true
- FLINK_HOME="${INSTALL_DIR}/flink-${FLINK_VERSION}"
- FLINK_MAJOR_MINOR="$(flink_major_minor "$FLINK_VERSION")"
- revalidate_python_constraint
;;
install_dir)
INSTALL_DIR="$(prompt_path_choice_interactive \
@@ -1152,7 +1168,6 @@ edit_plan_interactive() {
"$INSTALL_DIR" \
"/path/to/flink-install-dir")"
INSTALL_DIR="$(normalize_path "$INSTALL_DIR")"
- FLINK_HOME="${INSTALL_DIR}/flink-${FLINK_VERSION}"
;;
flink_home)
edit_prompt_flink_home
@@ -1174,6 +1189,8 @@ edit_plan_interactive() {
;;
esac
+ reconcile_plan_after_edit "$action"
+
edit_plan_dump_state "$state_file"
) || rc=$?
@@ -1411,12 +1428,15 @@ pip_install_quiet() {
}
setup_python_env() {
- # A venv the user opted to recreate (incompatible interpreter, see
- # revalidate_existing_venv) is deleted here, not at plan time, so a later
- # cancel never destroys it. Guard the rm to a real venv marker.
- if [[ "${RECREATE_VENV:-0}" == "1" && -f "$VENV_DIR/pyvenv.cfg" ]]; then
+ # Recreation is deferred until install time so cancelling the plan never
+ # destroys data. Require both the exact approved path and a real venv
+ # marker before removing anything.
+ if [[ -n "${RECREATE_VENV_PATH:-}" \
+ && "$VENV_DIR" == "$RECREATE_VENV_PATH" \
+ && -f "$VENV_DIR/pyvenv.cfg" ]]; then
ui_info "Recreating virtual environment: $VENV_DIR"
rm -rf "$VENV_DIR"
+ RECREATE_VENV_PATH=""
fi
if [[ ! -d "$VENV_DIR" ]]; then
@@ -1680,13 +1700,14 @@ resolve_python() {
# (see python_minor_ceiling), so editing either at the confirm screen can
# invalidate a Python setup that was already accepted — e.g. Python 3.12
# passes for 0.3.0 + Flink 2.2 but not after editing down to 0.2.1 or to
-# Flink 2.0. Called after a version edit; mirrors the enable_pyflink arm.
+# Flink 2.0. Called by reconcile_plan_after_edit after every menu edit.
# No-op unless PyFlink is enabled. Two independent checks:
# 1. PYTHON_BIN — the resolved interpreter; re-resolve if it no longer fits.
# 2. VENV_DIR — see revalidate_existing_venv; a reused venv keeps its own
# interpreter, which re-resolving PYTHON_BIN does not change.
revalidate_python_constraint() {
if [[ "$ENABLE_PYFLINK" != "Yes" ]]; then
+ RECREATE_VENV_PATH=""
return 0
fi
@@ -1704,22 +1725,35 @@ revalidate_python_constraint() {
# PYTHON_BIN — is what pip runs under. After a version edit that lowers the
# Python ceiling, an existing venv built on a now-too-new interpreter would
# still be reused and fail at `pip install` time. Catch it at plan time:
-# offer to recreate it (deferred to setup_python_env via RECREATE_VENV, which
-# only ever deletes a directory carrying a pyvenv.cfg marker) or point at a
-# different path. Non-interactive callers get an actionable error instead of a
-# mid-install failure. No-op unless VENV_DIR is an existing venv.
+# offer to recreate it (deferred to setup_python_env via RECREATE_VENV_PATH,
+# which binds approval to this exact venv) or point at a different path.
+# Non-interactive callers get an actionable error instead of a mid-install
+# failure. No-op unless VENV_DIR is an existing venv.
revalidate_existing_venv() {
- RECREATE_VENV=0
- [[ "$ENABLE_PYFLINK" == "Yes" && -n "${VENV_DIR:-}" ]] || return 0
+ if [[ "$ENABLE_PYFLINK" != "Yes" || -z "${VENV_DIR:-}" ]]; then
+ RECREATE_VENV_PATH=""
+ return 0
+ fi
local kind
kind="$(validate_venv_dir "$VENV_DIR")" || true
- [[ "$kind" == "venv" ]] || return 0
+ if [[ "$kind" != "venv" ]]; then
+ RECREATE_VENV_PATH=""
+ return 0
+ fi
if validate_python_bin "$VENV_DIR/bin/python"; then
+ RECREATE_VENV_PATH=""
return 0
fi
+ # A previous edit may re-run plan validation. Preserve an approval only
+ # while it still refers to this same incompatible venv.
+ if [[ "${RECREATE_VENV_PATH:-}" == "$VENV_DIR" ]]; then
+ return 0
+ fi
+ RECREATE_VENV_PATH=""
+
local venv_pv
venv_pv="$("$VENV_DIR/bin/python" -c 'import sys;
print(f"{sys.version_info.major}.{sys.version_info.minor}")' 2>/dev/null ||
printf 'unknown')"
ui_warn "Existing venv ${VENV_DIR} uses Python ${venv_pv}, incompatible
with Flink Agents ${FLINK_AGENTS_VERSION} + Flink ${FLINK_VERSION} (needs
>=3.10 and <3.$(python_minor_ceiling))"
@@ -1729,7 +1763,7 @@ revalidate_existing_venv() {
fi
if choose_install_method_interactive "Recreate ${VENV_DIR} with a
compatible interpreter?"; then
- RECREATE_VENV=1
+ RECREATE_VENV_PATH="$VENV_DIR"
else
prompt_and_validate_venv_dir "$VENV_DIR"
# The newly chosen path may itself be an incompatible venv.
@@ -1991,4 +2025,3 @@ if [[ "${FLINK_AGENTS_INSTALL_SH_NO_RUN:-0}" != "1" ]];
then
configure_verbose
main
fi
-
diff --git a/tools/test/helpers/load.bash b/tools/test/helpers/load.bash
index e2cae738..f46f9891 100644
--- a/tools/test/helpers/load.bash
+++ b/tools/test/helpers/load.bash
@@ -28,7 +28,7 @@ reset_install_sh_state() {
ENABLE_PYFLINK="Ask"
PYTHON_BIN=""
PYFLINK_ACTUALLY_ENABLED=0
- RECREATE_VENV=0
+ RECREATE_VENV_PATH=""
FLINK_VERSION="2.2.0"
FLINK_AGENTS_VERSION="0.3.0"
FLINK_SCALA_VERSION="2.12"
diff --git a/tools/test/unit/edit_plan_quote.bats
b/tools/test/unit/edit_plan_quote.bats
index 426b1d59..ac6ae60b 100644
--- a/tools/test/unit/edit_plan_quote.bats
+++ b/tools/test/unit/edit_plan_quote.bats
@@ -63,6 +63,7 @@ setup() {
VENV_DIR="/tmp/venv with space"
PYTHON_BIN="/usr/bin/python3"
FLINK_AGENTS_VERSION="0.2.0"
+ RECREATE_VENV_PATH="/tmp/venv with space"
local f="$BATS_TEST_TMPDIR/state"
edit_plan_dump_state "$f"
@@ -71,6 +72,7 @@ setup() {
INSTALL_FLINK=""; FLINK_VERSION=""; INSTALL_DIR=""; FLINK_HOME=""
FLINK_MAJOR_MINOR=""; ENABLE_PYFLINK=""; PYFLINK_ACTUALLY_ENABLED=0
VENV_DIR=""; PYTHON_BIN=""; FLINK_AGENTS_VERSION=""
+ RECREATE_VENV_PATH=""
# shellcheck disable=SC1090
source "$f"
@@ -85,4 +87,5 @@ setup() {
[ "$VENV_DIR" = "/tmp/venv with space" ]
[ "$PYTHON_BIN" = "/usr/bin/python3" ]
[ "$FLINK_AGENTS_VERSION" = "0.2.0" ]
+ [ "$RECREATE_VENV_PATH" = "/tmp/venv with space" ]
}
diff --git a/tools/test/unit/reconcile_plan_after_edit.bats
b/tools/test/unit/reconcile_plan_after_edit.bats
new file mode 100644
index 00000000..afa4f864
--- /dev/null
+++ b/tools/test/unit/reconcile_plan_after_edit.bats
@@ -0,0 +1,55 @@
+#!/usr/bin/env bats
+
+setup() {
+ load '../helpers/load'
+ load '../helpers/shim'
+ load_install_sh
+ reset_install_sh_state
+ shim_setup
+ VENV_DIR="$BATS_TEST_TMPDIR/no-venv"
+}
+
+fake_python() {
+ local name="$1"
+ local ver="$2"
+ shim_bin_script "$name" "
+case \"\$*\" in
+ *'sys.version_info.major'*) echo '$ver' ;;
+esac
+"
+}
+
+@test "reconcile_plan_after_edit: re-detects FLINK_HOME and revalidates
Python" {
+ local flink_home="$BATS_TEST_TMPDIR/flink-2.0.2"
+ mkdir -p "$flink_home/lib"
+ : > "$flink_home/lib/flink-dist-2.0.2.jar"
+ fake_python fake_py312 "3.12"
+ fake_python python3 "3.11"
+
+ INSTALL_FLINK="No"
+ FLINK_HOME="$flink_home"
+ FLINK_VERSION="2.2.1"
+ FLINK_MAJOR_MINOR="2.2"
+ ENABLE_PYFLINK="Yes"
+ FLINK_AGENTS_VERSION="0.3.0"
+ PYTHON_BIN="fake_py312"
+
+ reconcile_plan_after_edit flink_home
+
+ [ "$FLINK_VERSION" = "2.0.2" ]
+ [ "$FLINK_MAJOR_MINOR" = "2.0" ]
+ [ "$PYTHON_BIN" = "python3" ]
+}
+
+@test "reconcile_plan_after_edit: derives FLINK_HOME for managed installs" {
+ INSTALL_FLINK="Yes"
+ INSTALL_DIR="$BATS_TEST_TMPDIR/flink installs"
+ FLINK_VERSION="2.1.3"
+ FLINK_HOME="/stale/home"
+ FLINK_MAJOR_MINOR="2.2"
+
+ reconcile_plan_after_edit install_dir
+
+ [ "$FLINK_HOME" = "$INSTALL_DIR/flink-2.1.3" ]
+ [ "$FLINK_MAJOR_MINOR" = "2.1" ]
+}
diff --git a/tools/test/unit/revalidate_python_constraint.bats
b/tools/test/unit/revalidate_python_constraint.bats
index 354df87a..96bbfa0a 100644
--- a/tools/test/unit/revalidate_python_constraint.bats
+++ b/tools/test/unit/revalidate_python_constraint.bats
@@ -158,11 +158,11 @@ EOF
ENABLE_PYFLINK="Yes"
FLINK_AGENTS_VERSION="0.2.1"
VENV_DIR="$BATS_TEST_TMPDIR/venv311"
- RECREATE_VENV=0
+ RECREATE_VENV_PATH=""
run revalidate_existing_venv
[ "$status" -eq 0 ]
- [ "$RECREATE_VENV" = "0" ]
+ [ "$RECREATE_VENV_PATH" = "" ]
}
@test "revalidate_existing_venv: no-op when VENV_DIR is not an existing venv" {
@@ -173,3 +173,72 @@ EOF
run revalidate_existing_venv
[ "$status" -eq 0 ]
}
+
+@test "revalidate_existing_venv: binds recreation approval to the current venv
path" {
+ local old_venv="$BATS_TEST_TMPDIR/venv312"
+ fake_venv "$old_venv" "3.12"
+
+ ENABLE_PYFLINK="Yes"
+ FLINK_AGENTS_VERSION="0.2.1"
+ VENV_DIR="$old_venv"
+ is_promptable() { return 0; }
+ choose_install_method_interactive() { return 0; }
+
+ revalidate_existing_venv
+ [ "$RECREATE_VENV_PATH" = "$old_venv" ]
+}
+
+@test "revalidate_existing_venv: clears recreation approval after the venv
path changes" {
+ local old_venv="$BATS_TEST_TMPDIR/venv312"
+ local new_venv="$BATS_TEST_TMPDIR/venv311"
+ fake_venv "$old_venv" "3.12"
+ fake_venv "$new_venv" "3.11"
+
+ ENABLE_PYFLINK="Yes"
+ FLINK_AGENTS_VERSION="0.2.1"
+ VENV_DIR="$new_venv"
+ RECREATE_VENV_PATH="$old_venv"
+
+ revalidate_existing_venv
+ [ "$RECREATE_VENV_PATH" = "" ]
+}
+
+@test "setup_python_env: does not apply recreation approval to a different
venv" {
+ local approved_venv="$BATS_TEST_TMPDIR/approved-venv"
+ local selected_venv="$BATS_TEST_TMPDIR/selected-venv"
+ fake_venv "$approved_venv" "3.12"
+ fake_venv "$selected_venv" "3.11"
+ touch "$selected_venv/must-survive"
+
+ ENABLE_PYFLINK="Yes"
+ FLINK_AGENTS_VERSION="0.3.0"
+ FLINK_VERSION="2.0.2"
+ FLINK_MAJOR_MINOR="2.0"
+ VENV_DIR="$selected_venv"
+ RECREATE_VENV_PATH="$approved_venv"
+ create_venv() { fake_venv "$VENV_DIR" "3.11"; }
+ pip_install_quiet() { return 0; }
+
+ run setup_python_env
+ [ "$status" -eq 0 ]
+ [ -f "$selected_venv/must-survive" ]
+}
+
+@test "setup_python_env: recreates the exact venv path that was approved" {
+ local selected_venv="$BATS_TEST_TMPDIR/selected-venv"
+ fake_venv "$selected_venv" "3.12"
+ touch "$selected_venv/old-marker"
+
+ ENABLE_PYFLINK="Yes"
+ FLINK_AGENTS_VERSION="0.2.1"
+ FLINK_VERSION="2.0.2"
+ FLINK_MAJOR_MINOR="2.0"
+ VENV_DIR="$selected_venv"
+ RECREATE_VENV_PATH="$selected_venv"
+ create_venv() { fake_venv "$VENV_DIR" "3.11"; }
+ pip_install_quiet() { return 0; }
+
+ run setup_python_env
+ [ "$status" -eq 0 ]
+ [ ! -f "$selected_venv/old-marker" ]
+}
