weiqingy opened a new pull request, #986:
URL: https://github.com/apache/flink-agents/pull/986
Linked issue: #982
> Stacked on #979. The first two commits belong to that PR and are already
under review there. **Only the last commit, `[infra] Guard uv in e2e.sh and fix
the pytest log level in ut.sh`, is new here.** I'll rebase once #979 merges.
### 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.sh` breaks without `uv` on PATH
It invokes `uv` by bare name seven times and has no `command -v uv` guard
anywhere, so a machine without uv on PATH gets seven `uv: command not found`
lines and no hint about what to install.
It is the odd one out among the build scripts. `tools/lint.sh` and
`tools/ut.sh` both guard their uv usage and fall back to pip, and
`tools/build.sh` installs uv before using it. `tools/e2e.sh` does neither. It
only delegates to `tools/build.sh` conditionally, and that delegation is
skipped once `e2e-test/target` and `python/uv.lock` exist.
So it now probes once, after the build delegation so that a build performed
in the same run counts:
```bash
if command -v uv >/dev/null 2>&1; then
UV=(uv)
elif python3 -m uv --version >/dev/null 2>&1; then
UV=(python3 -m uv)
else
echo "Error: uv not found. Run tools/build.sh first, or install uv." >&2
exit 1
fi
```
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 `uv`
module, so preferring the module would break exactly those installs.
The module branch tests uv by *running* it rather than by importing it.
`uv/__init__.py` imports `find_uv_bin` without calling it, and the
five-directory search for the binary happens at call time inside
`uv/__main__.py`. So `import uv` succeeds 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 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. `tools/build.sh`
already does the same at its own wheel install.
#### `tools/ut.sh` passes a log level pytest rejects
The pip fallback passed `-o log_cli_level=${LOG_LEVEL:-OFF}` while the uv
branch in the same function passed `${LOG_LEVEL:-CRITICAL}`. `OFF` is not a
Python logging level name, so pytest rejects it during startup:
```
ERROR: 'OFF' is not recognized as a logging level name for 'log_cli_level'.
Please consider passing the logging level num instead.
```
That is exit 4, raised before any test is collected, which `ut.sh` then
reports through its "unknown error" arm. Aligning the fallback with the uv
branch lets the path run.
### Tests
CI behavior is unchanged. `LOG_LEVEL: INFO` is set at four workflow sites,
and the two Python jobs that fall back to the default take the uv branch, which
already used `CRITICAL`. 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:
| uv on PATH | module importable | binary findable | Selected | Exit |
| --- | --- | --- | --- | --- |
| yes | yes | yes | `uv` | 0 |
| yes | no | n/a | `uv` | 0 |
| no | yes | yes | `python3 -m uv` | 0 |
| no | no | n/a | error | 1 |
| no | yes | **no** | error | 1 |
The last row is the case that motivated testing the module by running it.
With an import-based probe it selected `python3 -m uv` and 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 are
`apache-flink~=2.3.0` being uninstallable on macOS arm64, since no
`apache-beam` wheel 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 -n` passes 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-needed`
- [x] `doc-not-needed`
- [ ] `doc-included`
### Was this patch authored or co-authored using generative AI tooling?
- [x] Yes
- [ ] No
--
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]
