This is an automated email from the ASF dual-hosted git repository. wenjin272 pushed a commit to branch release-0.3 in repository https://gitbox.apache.org/repos/asf/flink-agents.git
commit ea2458107c5936a254cb5322796d4bdc1863b7b1 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]> (cherry picked from commit 6f020c5061bc1b444fbdb832d57ee7430517a62a) --- 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" ] +}
