Repository navigation
[infra] Guard uv in e2e.sh and fix the pytest log level in ut.sh - #986
Merged
Merged
Conversation
wenjin272
approved these changes
Aug 9, 2026
wenjin272
left a comment
Contributor
There was a problem hiding this comment.
Thanks for taking this on. LGTM. Merging after resolving conflicts.
Two defects in fallback paths that CI never exercises. tools/e2e.sh invoked uv by bare name seven times with no guard anywhere, so a machine without uv on PATH got seven "command not found" lines and no indication of what to install. Unlike the sibling scripts it neither guards its uv usage the way lint.sh and ut.sh do, nor installs uv the way build.sh does. It only delegates to build.sh conditionally, and that delegation is skipped once e2e-test/target and uv.lock exist. Probe once instead, after the build delegation so that a build performed in this run counts, and prefer the binary on PATH. A standalone uv (curl installer, homebrew) provides a working binary and no importable module, so the PATH form has to win; the module is the fallback, and neither available is now a clear error rather than seven obscure ones. The probe tests the module by running it rather than by importing it, because uv/__init__.py imports find_uv_bin without calling it, so an import succeeds even when the binary it locates is absent. The pip install site also names its target environment. uv pip honors the VIRTUAL_ENV that uv's module entry point derives from the calling interpreter, so without --python .venv the fallback could install apache-flink outside the venv the tests actually run in. build.sh already does the same at its own wheel install. tools/ut.sh passed -o log_cli_level=OFF in its pip fallback while the uv branch passed CRITICAL. OFF is not a Python logging level name, so pytest rejected it and exited 4 during startup, before collecting anything. Aligning the two branches lets that path run: it now collects 375 tests where it previously collected none. Generated-by: Claude Code 2.1.226
weiqingy
force-pushed
the
982-build-script-fallbacks
branch
from
August 9, 2026 05:52
6fef839 to
dae8a7c
Compare
Collaborator
Author
|
@wenjin272 Thanks for the review. Rebased onto main now that #979 is in. The conflict was the squash: #979's two commits landed as one commit with a new SHA, so this branch and main were changing the same three files from different ancestries. I dropped those two commits with The PR is down to the single new commit, touching only |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked issue: #982
Purpose of change
Two defects in fallback paths that CI never exercises, which is why neither has surfaced before. Both were found while working on #979.
tools/e2e.shbreaks withoutuvon PATHIt invokes
uvby bare name seven times and has nocommand -v uvguard anywhere, so a machine without uv on PATH gets sevenuv: command not foundlines and no hint about what to install.It is the odd one out among the build scripts.
tools/lint.shandtools/ut.shboth guard their uv usage and fall back to pip, andtools/build.shinstalls uv before using it.tools/e2e.shdoes neither. It only delegates totools/build.shconditionally, and that delegation is skipped oncee2e-test/targetandpython/uv.lockexist.So it now probes once, after the build delegation so that a build performed in the same run counts:
The binary on PATH has to win over the module. A standalone uv, from the curl installer or homebrew, gives a working binary and no importable
uvmodule, so preferring the module would break exactly those installs.The module branch tests uv by running it rather than by importing it.
uv/__init__.pyimportsfind_uv_binwithout calling it, and the five-directory search for the binary happens at call time insideuv/__main__.py. Soimport uvsucceeds even when the binary is missing, and a probe built on it would select a module that then fails at all seven sites, relocating the very failure this change removes.The
uv pip installsite also names its target environment.uv piphonors theVIRTUAL_ENVthat uv's module entry point derives from the calling interpreter, so without--python .venvthe fallback could installapache-flinkoutside the venv the tests actually run in.tools/build.shalready does the same at its own wheel install.tools/ut.shpasses a log level pytest rejectsThe pip fallback passed
-o log_cli_level=${LOG_LEVEL:-OFF}while the uv branch in the same function passed${LOG_LEVEL:-CRITICAL}.OFFis not a Python logging level name, so pytest rejects it during startup:That is exit 4, raised before any test is collected, which
ut.shthen reports through its "unknown error" arm. Aligning the fallback with the uv branch lets the path run.Tests
CI behavior is unchanged.
LOG_LEVEL: INFOis set at four workflow sites, and the two Python jobs that fall back to the default take the uv branch, which already usedCRITICAL. The runners also provide uv on PATH, so the probe selects the same binary the script used before.The probe was exercised across every combination of binary and module availability:
uvuvpython3 -m uvThe last row is the case that motivated testing the module by running it. With an import-based probe it selected
python3 -m uvand exited 0, then failed at the first real call site.Argument passing was checked at all seven sites against the previous version with a stub on PATH, including the one inside a command substitution and the ones in
cd ... &&chains. Byte-identical under both probe branches.For
tools/ut.sh, the fallback path previously exited 4 with nothing collected. It now collects 375 tests and 293 pass locally. The rest areapache-flink~=2.3.0being uninstallable on macOS arm64, since noapache-beamwheel at the required version exists for that platform. That is environmental and unrelated to this change, so this fixes the first blocker on that path rather than making it green everywhere.bash -npasses on both scripts, and shellcheck reports the same finding set before and after, with nothing landing on the new block.API
No API change.
Documentation
doc-neededdoc-not-neededdoc-includedWas this patch authored or co-authored using generative AI tooling?