ci: build python bindings without pip build isolation - #2036
Conversation
|
👋 Hi NirWolfer! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Python test script now installs and upgrades the complete build dependency set. Package installation uses ChangesPython build
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/build |
|
🤖 CI Triage Agent — TL;DR: The "Run DL Python tests" stage failed at Full analysisSummary: Root cause: PR #2036 adds Implicated commit: [REDACTED:Hex High Entropy String] (PR #2036, "ci: build python bindings without pip build isolation"). The committed File: Suggested fix: Before building with Related: PR #2036 (#2036); prometheus plugin origin PR #1091, cmake build fix PR #1196.
|
|
🤖 CI Triage Agent — TL;DR: The "Run Python tests" stages fail because Full analysisSummary: Python wheel build (meson-python metadata generation) fails after a fully successful compile; C++ build/tests pass, only Root cause: meson-python cannot map an install target at Implicated commit: unknown — triggered by PR #2036 (commit [REDACTED:Hex High Entropy String]); the diff was not retrievable with available tools. The install rule creating File: Suggested fix: Prevent the prometheus-cpp subproject Related: none found (issue/PR search returned no matches for the error signature).
|
|
🤖 CI Triage Agent — TL;DR: The "Test Python" stage fails because Full analysisSummary: Root cause: meson-python requires every file with Implicated commit: PR #2036 "ci: build python bindings without pip build isolation" (head commit [REDACTED:Hex High Entropy String]). The problematic subproject install originates in File: Suggested fix: Prevent the prometheus-cpp shared libraries from landing in an unmapped Related: PR #2036 (#2036); prometheus plugin origin PR #1091; histogram change #1914.
|
|
🤖 CI Triage Agent — TL;DR: The build compiled and pushed the Docker image successfully; the failure is in the "Allocate DL EP Environment" stage where Full analysisSummary: Stage 252 "Allocate DL EP Environment" failed because a SLURM Root cause: The job was submitted at Implicated commit: none — commit File: Jenkins pipeline SLURM allocation step ( Suggested fix: Retry the build — this is a transient cluster-capacity failure. To harden the pipeline: (1) add automatic retry/backoff around the Related: none |
|
/build |
|
🤖 CI Triage Agent — TL;DR: The wheel build compiles fully but meson-python aborts packaging with Full analysisSummary: Root cause: meson-python errors: Implicated commit: Configuration mismatch, not a single code line. Relevant: File: Suggested fix: Ensure the prometheus-cpp static archives are never Related: PR #2036 (#2036); prometheus meson history #1914, #1091.
|
|
🤖 CI Triage Agent — TL;DR: All "Build image" stages failed because Full analysisSummary: The Dockerfile's final Root cause: In the meson-python wheel build the prometheus-cpp CMake subproject is configured with Implicated commit: PR #2036 (build c17f15d…) — "ci: build python bindings without pip build isolation." Author not shown in the fetched logs. File: Suggested fix: Prevent the prometheus-cpp static archives from being installed into the wheel. Concretely, either:
Since only the wheel path breaks, gating the static-lib install on whether meson-python is driving the build (or simply not installing prometheus |
|
🤖 CI Triage Agent — TL;DR: The "Run DL Python tests" stage failed because Full analysisSummary: Root cause: PR #2036 makes the CI install the Python bindings with Implicated commit: [REDACTED:Hex High Entropy String] (PR #2036, "ci: build python bindings without pip build isolation"). The underlying installable-static-lib behavior comes from File: Suggested fix: Prevent the prometheus-cpp static archives from being included in the meson install/wheel set. Concrete options:
Related: PR #2036 (#2036); prometheus plugin origin PR #1091, build fix #1196.
|
|
🤖 CI Triage Agent — TL;DR: All 6 "Test Python" stages failed deterministically with Full analysisSummary: The "Test Python" pipeline stage fails during Root cause: The prometheus-cpp subproject builds static libraries ( Implicated commit: PR #2036 "ci: build python bindings without pip build isolation" (head commit [REDACTED:Hex High Entropy String]). The offending install declaration lives in the vendored prometheus-cpp subproject's generated meson build. File: Suggested fix: Ensure the prometheus-cpp static libraries are not part of the install manifest that meson-python consumes. Concretely, set
|
|
🤖 CI Triage Agent — TL;DR: The "Run Python tests" stages failed during Full analysisSummary: Both Root cause: meson-python failed with Implicated commit: PR #2036, head commit File: Suggested fix: Keep prometheus-cpp built as shared libraries by setting Related: none found (issue/PR search for the meson-python wheel-mapping error returned no matches).
|
|
🤖 CI Triage Agent — TL;DR: The build compiled cleanly; the job failed in the "Allocate DL EP Environment" stage because a Slurm Full analysisSummary: DL EP GPU test failed at cluster resource allocation, not in build or tests. Root cause: In stage 200 ( Implicated commit: unknown — not caused by commit c17f15d/PR #2036; the failure is external Slurm capacity. File: Jenkins pipeline Slurm allocation step ( Suggested fix: Re-run the job when the Related: none |
11ce17f to
5b5c514
Compare
|
/build |
NirWolfer
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Line 17: Update the pyproject.toml build-system requirements to provide a
compatible Torch dependency for isolated builds when build_nixl_ep is enabled,
or explicitly reject isolated EP builds with a clear configuration check.
Preserve the existing default build behavior and do not rely on the
--no-build-isolation wheel path to satisfy the isolated PEP 517 contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 4fc578b9-b993-4a1f-b361-86b4f18ce6cc
📒 Files selected for processing (1)
pyproject.toml
ovidiusm
left a comment
ovidiusm
left a comment
There was a problem hiding this comment.
This is incorrect, please do not merge
To make the build work without build isolation in CI, you need to install manually the build deps so modify I don't remember exactly why this command is different from the one above, but keep in mind that we need to make both CI pass this and the verification team scripts pass the run @Bohatchuk |
5b5c514 to
533d770
Compare
|
🤖 CI Triage Agent — TL;DR: The "Run Nixlbench tests" stage crashed with SIGSEGV (exit 139) in the UCCL backend during the Full analysisSummary: Jenkins stage 427 "Run Nixlbench tests" (ucx-master image variant) failed when Root cause: The UCCL backend crashes on teardown (the benchmark row printed and both processes logged "Destroying Engine…/Engine destroyed", then task 0 segfaulted — a race/crash in UCCL engine cleanup, not a hang or timeout). This is the known-flaky UCCL nixlbench issue #1999. Commit #2000 attempted to mitigate it but only commented out the ETCD-based UCCL loop ( Implicated commit: 7a244b9 "CI: Skip UCCL nixlbench tests (#2000)" by ovidiusm — incomplete skip. Underlying crash is in the UCCL backend introduced/optimized in [REDACTED:Hex High Entropy String] (#1271) / [REDACTED:Hex High Entropy String] (#895) by Pravein Govindan Kannan. File: Suggested fix: Comment out or gate the ASIO UCCL loop (lines 95-103) the same way the ETCD UCCL loop (lines 122-132) was disabled, referencing issue #1999, so no UCCL nixlbench transfer tests run until the segfault is fixed. Longer term, root-cause the UCCL engine teardown crash in the UCCL plugin (destroy/engine cleanup path). Note this is a code crash, not a wall-clock issue — do not raise any timeout. Related: Issue #1999 (UCCL nixlbench test hangs); PR #2000 (partial skip); UCCL PRs #1271, #895, #1428. Side note: stage 176
|
|
🤖 CI Triage Agent — TL;DR: The "Run Python tests" stages failed instantly with a Full analysisSummary: Both parallel "Run Python tests" stages (node 267 master, node 270 v1.22.x) of Root cause: Implicated commit: none — infrastructure failure, not code. (PR commit 10ccfaa is not responsible.) File: N/A (failure originates in the Jenkins/pyxis runtime layer, not repo source). Invocation line: Suggested fix: Re-run the build — this is almost certainly a transient runner fault. If it recurs: investigate the CI executor node ( Related: none found. |
|
🤖 CI Triage Agent — TL;DR: All three parallel "Build Wheel" stages were killed by a pipeline timeout (exit 143) because the Full analysisSummary: Root cause: In every failing stage the last application output is Implicated commit: none — this is an infrastructure/registry hang, not a source regression. ( File: contrib/build-container.sh:425 (the Suggested fix: Retry the build first — a transient Artifactory/network stall is the most likely cause. To make this class of failure non-fatal and diagnosable going forward: (1) pre-pull the wheel-base image with an explicit timeout and bounded retries (e.g. wrap the pull in Related: none found.
|
|
🤖 CI Triage Agent — TL;DR: The Full analysisSummary: Stage Root cause: The Implicated commit: File: Suggested fix: Rebase PR #2036 onto a main that includes commit 029a854 so the internal Related: PR #1961 (internal-mirror switch); PR #2036 (this build).
|
pip provisions [build-system].requires into a fresh PEP 517 build env on every run. The pinned torch==2.11.* can never be satisfied by the image's torch (an NVIDIA nightly, e.g. 2.13.0a0), so pip re-downloaded torch plus its CUDA dependency closure -- several GB -- on each invocation. That normally takes ~2 min but stalls for 10-30 min when the index or network degrades, tripping the stage timeout with exit 143; it caused 10 CI failures between Jul 1 and Aug 3. CI does not need an isolated build environment, so build with --no-build-isolation and provision the build dependencies explicitly. [build-system].requires is left untouched, so the isolated-build contract for consumers building from source, including torch for nixl_ep, is unchanged. Install those dependencies with --upgrade. Without it pip treats an already-present version as satisfying the requirement and leaves it in place: the images ship meson 1.3.2, which is what the failing runs used. The system-python branch also installed only tomlkit and relied on the image for the rest, so both branches now install the same set. Signed-off-by: NirWolfer <nwolfer@nvidia.com>
533d770 to
dca0298
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/build |
|
🤖 CI Triage Agent — TL;DR: The "Test Python" stages failed because Full analysisSummary: All "Test Python" stages on the Python-3.12 images (cuda12.9-ubuntu24.04 and pytorch26.06, both x86_64 and aarch64) failed during build-dependency installation; the ubuntu22.04/Python-3.10 images passed. Root cause: Implicated commit: PR #2036 ("ci: build python bindings without pip build isolation") — the branch version of File: Suggested fix: In the PR's Related: #2036 |
|
🤖 CI Triage Agent — TL;DR: The "Run DL Python tests" stage failed at the build-dependency Full analysisSummary: Root cause: The base Docker image ships PyYAML 6.0.1 installed via Debian's apt (no pip RECORD file). PR #2036 changed the build-dependency install to run Implicated commit: PR #2036 head, File: Suggested fix: In the system-Python install path, avoid forcing an upgrade over the apt-managed PyYAML. Options:
Related: #2036
|
|
🤖 CI Triage Agent — TL;DR: The AWS Batch job failed during the "EFA Python Tests" setup because Full analysisSummary: The "EFA Python Tests" step aborted when a Root cause: Log line Implicated commit: unknown (the failing File: the EFA Python Tests setup command in the AWS-batch test wrapper / GHA workflow that runs Suggested fix: Make the PyYAML dependency install robust against the Debian-managed package. Options, in order of preference:
Related: none found (no matching issues/PRs).
|
|
🤖 CI Triage Agent — TL;DR: The "Run Python tests" stage failed during the build-dependency Full analysisSummary: Root cause: The install command executed was Implicated commit: PR #2036 ("ci: build python bindings without pip build isolation") — the change adds File: Suggested fix: Remove the Related: #2036
|
--upgrade alone fails on the Python 3.12 images: they carry a Debian
packaged PyYAML 6.0.1 with no pip RECORD, and the newer pip there treats
the uninstall as fatal rather than warning:
ERROR: Cannot uninstall PyYAML 6.0.1, RECORD file not found.
error: uninstall-no-record-file
--ignore-installed skips the uninstall path entirely and installs into
dist-packages, which precedes the distro location on sys.path. meson is
exposed the same way -- Ubuntu 24.04 ships the 1.3.2 that the failing
runs used -- so restricting the flag to PyYAML would only move the
failure one package along.
Signed-off-by: NirWolfer <nwolfer@nvidia.com>
|
/build |
2 similar comments
|
/build |
|
/build |
|
🤖 CI Triage Agent — TL;DR: The build itself succeeded; the "Allocate DL EP Environment" stage failed because a Slurm Full analysisSummary: Slurm node allocation on partition Root cause: In stage 156, Implicated commit: none — failure is unrelated to commit [REDACTED:Hex High Entropy String]; it is an environment allocation timeout. File: N/A (Jenkins pipeline Suggested fix: Re-run the build — this is a transient cluster-capacity failure. If GB200 ( Related: none found. Note: the
|
Expose the UUID utility's byte generator so trace and span IDs no longer require temporary UUID objects. Signed-off-by: Efraim Eygin <eeygin@nvidia.com>
3 participants
Summary
pip provisions
[build-system].requiresinto a fresh PEP 517 build environment on every run. The pinnedtorch==2.11.*can never be satisfied by the torch already in the CI images (an NVIDIA nightly, e.g.2.13.0a0+8145d630e8.nv26.6), so pip re-downloaded torch plus its CUDA dependency closure - several GB - on each invocation.That step normally completes in ~2 min, but stalls for 10-30 min when the index or network degrades, tripping the stage timeout and aborting with exit 143. It caused 10 CI failures between Jul 1 and Aug 3 across
nixl-ci-gpu,nixl-ci-non-gpuandnixl-ci-dl-gpu.CI does not need an isolated build environment, so this builds with
--no-build-isolationand provisions the build dependencies explicitly.[build-system].requiresis left untouched, so the isolated-build contract for consumers building from source - includingnixl_ep's torch requirement - is unchanged.Changes
.gitlab/test_python.shonly:--no-build-isolationto thepip install .invocation--upgrade, in a single branchWhy
--upgrademattersWithout it pip treats an already-present version as satisfying the requirement and leaves it in place. The images ship meson 1.3.2, and that is what the failing runs actually used:
contrib/Dockerfilealready installs these same packages with--upgrade, so this brings the CI script in line with it.The two branches previously diverged - the venv branch installed the full set while the system-python branch installed only
tomlkitand relied on the image for the rest. Both now install the same set, so the build no longer depends on image contents.Verification
bash -npasses. The stall is environment-dependent and not locally reproducible, so confirmation is a green CI run - specifically that the isolated build environment is no longer provisioned, and that with an upgraded meson the wheel builds without the{prefix}/libmapping error.Summary by CodeRabbit