feat(build): Add CI test that builds+tests the wheels - #78
Conversation
|
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:
📝 WalkthroughWalkthroughAdds a composite GitHub Action to stamp and build a single nemo-platform wheel, integrates it into CI for nightly wheel testing, refactors the release workflow to use the action, and adds a CLI integration test script. ChangesWheel build action and CI/release integration
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/actions/build-nemo-platform-wheel/action.yaml:
- Around line 171-178: The script currently does an unsafe rm -rf on
"${abs_out_dir}" which can delete arbitrary absolute paths; before running rm
-rf validate and guard abs_out_dir by (1) rejecting empty or "/" values and
exiting non‑zero, and (2) ensuring abs_out_dir is inside the SOURCE_ROOT path
(e.g., check that abs_out_dir starts with "$(cd "${SOURCE_ROOT}" && pwd)/" or
equivalent) so caller-provided OUT_DIR cannot point outside source tree; perform
these checks after computing abs_out_dir and only then run rm -rf
"${abs_out_dir}".
In @.github/workflows/ci.yaml:
- Around line 213-216: In the wheel-test job update the checkout step (uses:
actions/checkout@v6) to disable credential persistence by adding with:
persist-credentials: false and pin the action to its full commit SHA; likewise
replace uses: actions/download-artifact@v8 and uses: actions/upload-artifact@v6
with their respective full commit SHAs to avoid floating tags—locate the
checkout, download-artifact, and upload-artifact steps by those exact uses
strings and update them accordingly.
🪄 Autofix (Beta)
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: CHILL
Plan: Enterprise
Run ID: 729eeeef-86e9-4daa-bd13-59882a20c6ef
📒 Files selected for processing (4)
.github/actions/build-nemo-platform-wheel/action.yaml.github/workflows/ci.yaml.github/workflows/release-bundle.yamlscript/test-nemo-cli.sh
|
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 @.github/workflows/ci.yaml:
- Around line 223-231: The workflow uses a non-existent context property
github.run_started_at when setting RUN_STARTED_AT; replace RUN_STARTED_AT: ${{
github.run_started_at }} with a valid context such as RUN_STARTED_AT: ${{
github.event.workflow_run.run_started_at }} (or another appropriate event field
for your trigger) so the subsequent ts="$(date -u -d "${RUN_STARTED_AT}"
+%Y%m%d%H%M%S)" call in the run step gets a real ISO-8601 timestamp; update the
env assignment and verify stamp_sdk_version.py invocation still receives the
expected 14-digit string.
🪄 Autofix (Beta)
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: CHILL
Plan: Enterprise
Run ID: 14ac5ae2-d993-4d6d-9f49-87297d99bfaa
📒 Files selected for processing (1)
.github/workflows/ci.yaml
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 `@script/test-nemo-cli.sh`:
- Around line 81-85: The readiness loop currently only checks for the "default"
workspace via the jq filter '.data[]? | select(.name == "default")' which can
yield false positives; update the check used in the until condition (the call to
`nemo workspaces list -f json` and its jq predicate) to assert that both
"default" and "system" workspaces exist (e.g., change the jq expression to test
presence of both names or to count/match both entries) so the loop only succeeds
when both seeded workspaces are present.
🪄 Autofix (Beta)
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: CHILL
Plan: Enterprise
Run ID: 3a33d091-2d7a-4304-a3ea-c719825c70d2
📒 Files selected for processing (1)
script/test-nemo-cli.sh
bb0954a to
db49d62
Compare
mckornfield
left a comment
There was a problem hiding this comment.
two minors and a major, but probably fine (can stamp after)
f39e6d0 to
4ff125f
Compare
…-in-fresh-env-smoke-test-nemo-platform Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
* squash Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> * self code review Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> --------- Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> Signed-off-by: Alex Ray <alray@nvidia.com>
What
Adds a CI job that builds and tests every shipping wheel —
nemo-platformandnemo-platform-plugin— across every supported Python (3.11, 3.12, 3.13). Each row builds the wheel with the exact same code path the release workflow uses, installs it into an isolated env, and exercises it end-to-end. Today CI only runs against the editable workspace install, so wheel-install breakages (missingpolicy.wasm, vendor-metadata drift, entry-point typos, namespace-package gotchas, brokenRequires-Dist, generated-extra mismatches) only surface on manual post-release smoke checks.The build itself is factored into a new composite action,
.github/actions/build-nemo-platform-wheel, which is also called byrelease-bundle.yaml'sbuild-sdksmatrix. CI test wheels and published wheels now come out of one code path.Shape
wheel-testmatrix ispackage: [nemo-platform, nemo-platform-plugin] × python-version: [3.11, 3.12, 3.13]= 6 rows.fail-fast: falseso a single-row failure doesn't hide the others' logs.wheel-test-aggregateis a single-job umbrella that succeeds iff every matrix row succeeded. Branch protection should pin this check name (Wheel build + test); the matrix can grow without anyone touching protection settings.stamp_sdk_version.py,uv build --wheel, single-wheel guard,rm -rfsafety check on caller-providedout-dir). Stamping always runs regardless of caller, withcadence: nightly+ sentinel epoch19700101000000for CI tests so the test wheel version is well-formed PEP 440 dev but obviously synthetic.Per-package test surface
nemo-platform:uv tool install ./*.whl[services], thenscript/test-nemo-cli.shboots the platform vianemo services run(no--servicesfilter — every bundled service module loads, ~17 services + 4 controllers) and pollsnemo workspaces list -f jsonuntil bothdefaultandsystem(the platform-seeded workspaces) appear, with SIGTERM-then-SIGKILL teardown of the whole process group via bash job control (set -m).nemo-platform-plugin: library-only (no[project.scripts]), souv venv+uv pip install+python -c "import nemo_platform_plugin; import nemo_platform_plugin.cli; import nemo_platform_plugin.commands". That still catches the bug classes a wheel-test is supposed to catch (broken pyproject.toml, brokenRequires-Dist,hatch_build.pyregressions, namespace-package gotchas).script/test-nemo-cli.shis install-method agnostic — anywherenemois on PATH (uv tool installfrom a wheel,uv syncin dev,pipx, distro package), the script can run and prove the CLI works.What this does NOT do (yet)
Wheel build + testis set up to be the single pin point; flipping branch-protection-required is a one-line admin change once this has been green for a few working days.release-stable.yaml. That's the second admin step: extend the stable-release dispatch to fail ifWheel build + testdidn't pass on the source SHA.Platform-Deploy/.github/workflows/release-sdk.yamlstill has its own inline build path that drifts from this action; migrating it to consumebuild-nemo-platform-wheel@<sha>is a Platform-Deploy PR.lint.Coderabbit feedback addressed
rm -rf "${abs_out_dir}"in the action now rejects empty//paths and requires the resolved path to live inside source-root or$GITHUB_WORKSPACE.defaultandsystemworkspaces (the entities service seeds them back-to-back, so missing one is a real signal that init partially failed).github.run_started_atreference removed in favor of the sentinel epoch (the context property doesn't exist).actions/checkout@v6etc. andpersist-credentials: falsedeferred — every other job inci.yamluses major-version tags, so policy-pinning the wheel-test job alone would be inconsistent. Belongs in a separate repo-wide PR.Verification
yaml.safe_load.script/test-nemo-cli.shisshellcheckandbash -nclean.Tracking
docs/plans/aircore-682-wheel-test.mdSigned-off-by: Matthew Grossman mgrossman@nvidia.com