Repository navigation
chore(release): backport #15012 and #15019 to release/1.5.0 - #15027
Conversation
Signed-off-by: Harrison King Saturley-Hall <hsaturleyhal@nvidia.com> (cherry picked from commit 7d9cdd5)
| builder_name: b-${{ github.run_id }}-${{ github.run_attempt }} | ||
| inline_compliance: true | ||
| build_timeout_minutes: 45 | ||
| build_timeout_minutes: 90 # TEMPORARY CHANGE - TODO: INVESTIGATE AND SPEEDUP SLOW BUILDS |
There was a problem hiding this comment.
The inline note only narrates that this timeout increase is temporary and should be investigated; the changed value already records the operative configuration, while the comment preserves no lasting constraint or actionable context.
🤖 AI Fix
Remove the inline temporary/TODO comment and leave the timeout value unchanged.
| python -m pip download \ | ||
| --only-binary=:all: \ | ||
| --no-deps \ | ||
| --no-index \ |
There was a problem hiding this comment.
The 0.12.0 wheel is fetched from NVIDIA's index only for planner and framework-runtime targets, but the frontend target also runs on Python 3.12 and installs the top-level ai-dynamo wheel, which requires aisimulate==0.12.0. The PR documents PyPI's artifact as only a resolver sdist, so the frontend image never receives the required binary wheel and its AIC smoke path cannot import aiconfigurator_core.
🤖 AI Fix
Stage the NVIDIA AISimulate wheel for the frontend target and install that local wheel before installing the top-level Dynamo wheel.
There was a problem hiding this comment.
Premise does not hold. Verified by execution. The public PyPI artifact for aisimulate==0.12.0 is a wheel-stub redirector, not a dead end. A plain pip install aisimulate==0.12.0 against the default index builds through wheel_stub.buildapi, pulls the real 83 MB abi3 wheel from https://pypi.nvidia.com/, installs it and imports. The frontend target needs no staging change.
Measured: clean Python 3.12 venv, default index only, plus the sdist that proves the mechanism
$ python -m pip install --no-cache-dir "aisimulate==0.12.0"
Building wheels for collected packages: aisimulate
Building wheel for aisimulate (pyproject.toml): finished with status 'done'
Created wheel for aisimulate: filename=aisimulate-0.12.0-cp311-abi3-macosx_11_0_arm64.whl size=83827766
Successfully built aisimulate
$ python -c "import aisimulate; print(aisimulate.__file__)"
.../site-packages/aisimulate/__init__.py
The sdist contains only four files. Its pyproject.toml is:
[build-system]
requires = ["wheel-stub"]
build-backend = "wheel_stub.buildapi"
[tool.wheel_stub]
index_url = "https://pypi.nvidia.com/"The staging step is different because it passes --only-binary=:all:, which refuses the stub. That is the exact reason the --no-index plus --find-links change in this hunk is needed there and only there. The staging command, run as written from this head, resolves a real wheel for both build architectures:
| target | wheel | size |
|---|---|---|
manylinux_2_28_x86_64 |
aisimulate-0.12.0-cp311-abi3-manylinux_2_28_x86_64.whl |
83.6 MB |
manylinux_2_28_aarch64 |
aisimulate-0.12.0-cp311-abi3-manylinux_2_28_aarch64.whl |
83.3 MB |
| dependencies = [ | ||
| "ai-dynamo-runtime==1.5.0", | ||
| "aisimulate==0.12.0.dev2; python_version >= '3.11' and python_version < '3.14'", | ||
| "aisimulate==0.12.0; python_version >= '3.11' and python_version < '3.14'", |
There was a problem hiding this comment.
The frontend image installs this meta wheel with dependency resolution, but the new release's full AISimulate wheel is only staged from NVIDIA's index in wheel_builder for planner/runtime targets; target == "frontend" is excluded. The default PyPI resolver therefore sees only the documented resolver sdist (or no usable binary) when building the frontend for Python 3.11-3.13, causing the frontend image build to fail instead of installing this required dependency. Stage the NVIDIA wheel for the frontend target and make it an input to that image's uv pip install resolution.
🤖 AI Fix
Include the frontend target in AISimulate wheel staging and pass the staged local wheel or wheelhouse as a resolver source when frontend.Dockerfile installs ai-dynamo.
There was a problem hiding this comment.
Premise does not hold. Verified by execution. See the reply on the wheel_builder.Dockerfile thread. The public PyPI artifact for aisimulate==0.12.0 is a wheel-stub redirector to https://pypi.nvidia.com/, so the frontend resolver does get the real wheel. A plain pip install aisimulate==0.12.0 against the default index installed the 83 MB abi3 wheel and imported it in a clean Python 3.12 environment. No frontend staging change is needed.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approved. Faithful backport, no findings
Both picks are byte-identical to their source merge commits. Every declaration, the container index fix, the three new assertions and the documentation bump are all present. No findings at P0, P1, P2 or P3.
Note on self-review: I authored the source pull request for one of the two picks (#15019), so treat my approval of that one line as partly self-review.
Two of the three review-agent comments rest on a premise that does not hold. I replied on both threads with the measurement.
Fidelity: stable patch IDs and a diff of the two diffs
Reviewed at head 318c4d5a8b. The author merged release/1.5.0 into the branch while I was verifying, so the head moved from 6a629f14 to 318c4d5a. That merge brought in unrelated release-branch commits and changed nothing in this pick. I re-ran every check at the new head. Against the new release tip 378d3dc5c0 the whole branch diff is still 21 files, 50 insertions and 40 deletions, and its stable patch ID equals the two source patches concatenated (b5e10454da9f27d97884bd04f5a2925373575281).
| source merge commit | backport commit | stable patch ID |
|---|---|---|
7d9cdd5f (#15012) |
2ad96f8a |
9184da6b1a5e3d7fd85482a8778c80e8b5937f73 |
a28ecfb6 (#15019) |
6a629f14 |
245d2ae8bf41ea59a02f3b5c7d16126bbdc3d20d |
Diffing the source diff against the backport diff line by line:
- #15019: zero differing lines. The patch is byte-identical.
- #15012: 40 differing lines, all mechanical. Eight are blob index hashes. Two are hunk header offsets,
pyproject.tomlat 56 against 54 andwheel_builder.Dockerfileat 635 against 629. One is the trailing context label on a hunk header, because the nearest preceding line on this branch is a differentCOPYline. Every added and removed line is identical.
The source pull request #15012 landed on main as a squash, so its three commits are one commit there. All three pieces are in this pick: the version bump, the index fix with its three new assertions, and the documentation bump.
Assumptions on the release branch: six declarations, three lockfiles, one checksum
Every place that names the dependency at this head:
Cargo.toml:57 aisimulate-core = { version = "=0.12.0" }
lib/bindings/python/Cargo.toml:63 aisimulate-core = { version = "=0.12.0", optional = true, features = ["python"] }
pyproject.toml:17 aisimulate==0.12.0
benchmarks/pyproject.toml:42 aisimulate==0.12.0
container/deps/requirements.aisimulate.txt:5 aisimulate==0.12.0
tests/dependencies/test_aiconfigurator_consistency.py:96 "version": "=0.12.0"
A repository-wide search for the previous version string returns nothing:
$ git grep -c -E "0\.12\.0[.-]dev" HEAD | wc -l
0
The release branch holds no extra pin. The set of files that name the dependency on main at 7d9cdd5f and on release/1.5.0 before the pick differs by two files only, components/src/dynamo/mocker/main.py and lib/mocker/src/loadgen/weka.rs, neither of which exists on the release branch and neither of which carries a version. container/context.yaml has no key for this dependency, so there is no per-device override to fall out of step.
All three lockfiles carry the same checksum, and it matches the crate published on crates.io:
$ grep -A3 'name = "aisimulate-core"' Cargo.lock lib/bindings/kvbm/Cargo.lock lib/bindings/python/Cargo.lock
version = "0.12.0" (all three)
checksum = "7d266db788afdb866421aad39f8fbc7096dba9517105ff7d770ee83ad70f82bb" (all three)
$ curl -sS https://static.crates.io/crates/aisimulate-core/aisimulate-core-0.12.0.crate | shasum -a 256
7d266db788afdb866421aad39f8fbc7096dba9517105ff7d770ee83ad70f82bb
The lockfiles are consistent after the pick:
$ cargo metadata --locked --no-deps --format-version 1 # workspace OK
$ cargo metadata --locked --no-deps ... lib/bindings/python/Cargo.toml # python bindings OK
$ cargo metadata --locked --no-deps ... lib/bindings/kvbm/Cargo.toml # kvbm bindings OK
The staging command, run as written from this head, for both build architectures
$ python3 -m pip download --only-binary=:all: --no-deps --no-index \
--platform manylinux_2_28_x86_64 --python-version 3.12 --abi abi3 \
--find-links https://pypi.nvidia.com/aisimulate/ \
--requirement container/deps/requirements.aisimulate.txt
Saved aisimulate-0.12.0-cp311-abi3-manylinux_2_28_x86_64.whl (83.6 MB)
$ ... --platform manylinux_2_28_aarch64 ...
Saved aisimulate-0.12.0-cp311-abi3-manylinux_2_28_aarch64.whl (83.3 MB)
The index serves exactly three files for this version, and two of them are the Linux wheels the multi-architecture build needs. The public index carries only a 20 KB wheel-stub sdist for this version, which --only-binary=:all: refuses. That is what the --no-index and --find-links pair in this pick fixes.
Tests: they run here, the lanes select them, and a mutation turns them red
Run on this head, Python 3.12, from a worktree at the head commit:
$ pytest tests/dependencies/test_aisimulate_consistency.py \
tests/dependencies/test_aiconfigurator_consistency.py -q \
-k 'not installed_aisimulate and not preserves_aic_import_namespaces'
5 passed, 2 deselected
The two deselected cases need AISimulate 0.12.0 itself installed, which my environment does not have. They are left to CI, as the description says.
Marker selection, measured by running the expression rather than reading it:
| lane | marker expression | collected |
|---|---|---|
planner CPU test, pr.yaml:985 |
pre_merge and planner and gpu_0 |
5 of 29 |
base parallel lane, pr.yaml:1057 |
pre_merge and (parallel or defaulted) and not (vllm or sglang or trtllm) and (gpu_0) |
16 of 29 |
test_container_stages_the_published_aisimulate_wheel is in both.
Mutation control. Each mutation was applied to this head, the tests were run, then the file was restored:
| mutation | result |
|---|---|
delete --no-index and --find-links from the Dockerfile |
test_container_stages_the_published_aisimulate_wheel FAILED |
set the requirement back to 0.12.0.dev2 |
2 failed |
set the workspace crate back to =0.12.0-dev.2 |
test_dynamo_pins_matching_published_aisimulate_releases FAILED |
The workflow file still parses, and the documentation lint is clean:
$ python -c "import yaml; yaml.safe_load(open('.github/workflows/post-merge-ci.yml'))" # OK
$ python docs/fern/scripts/docs_lint.py --scan docs
files scanned: 436 | findings: 8 | errors: 0 # the 8 warnings are pre-existing
The timeout change lands on the frontend-build job and on that job only. planner-build above it still reads 45.
Where verification stopped
- I did not build a container image. The staging command was verified by running it on macOS with an explicit
--platformfor each Linux target, which downloads the same file the build would. - The two environment-dependent test cases were not run, because installing AISimulate 0.12.0 into the shared environment would disturb it.
- The build timeout change cannot be verified by execution at all. Whether 90 minutes is enough is an observation about build times, not a property of the diff.
- The image build jobs on this pull request were still running when I finished. I did not wait for them.
There was a problem hiding this comment.
Previously reported defects still present:
- Original discussion: The inline temporary/TODO comment on
build_timeout_minutes: 90remains unchanged. It repeats the configuration change without recording a lasting constraint or actionable rationale.
Summary
7d9cdd5fd0e9a0c7bb44c05ed0f2eff89b34947d) ontorelease/1.5.0aisimulateand Rustaisimulate-corereferences to 0.12.0a28ecfb6988563bb617cd7fe39488b712e98fb99) to temporarily raise the post-merge frontend build timeout from 45 to 90 minutesSource PRs: #15012, #15019
Source commits:
7d9cdd5fd0e9a0c7bb44c05ed0f2eff89b34947d,a28ecfb6988563bb617cd7fe39488b712e98fb99Backport commits:
2ad96f8a7d2cf2f9f3dcc7d89c485a2a3bb25538,6a629f1436f7d614b4c143af6c7e755263b8431bValidation
git diff --check github/release/1.5.0...HEADpytest -q tests/dependencies/test_aisimulate_consistency.py -k 'not installed_aisimulate'(4 passed, 1 deselected)pytest -q tests/dependencies/test_aiconfigurator_consistency.py -k 'not aisimulate_wheel_preserves_aic_import_namespaces'(1 passed, 1 deselected)python docs/fern/scripts/docs_lint.py --scan docs(0 errors; 8 pre-existing warnings)cargo metadata --locked --no-deps --format-version 1cargo metadata --locked --no-deps --format-version 1 --manifest-path lib/bindings/python/Cargo.toml.github/workflows/post-merge-ci.yml;actionlintis unavailable locallyThe wheel namespace compatibility assertion requires AISimulate 0.12.0. The local shared environment contains an editable AISimulate 0.1.0 checkout, so that environment-specific assertion is left for CI.