fix: allow NAT prereleases in agentic base image - #1123
Conversation
NVBug: 6557144 Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
df1473f to
75e9b4c
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughNAT installation commands now allow prerelease dependencies and use targeted extras. Docker and local requirements pin the NVIDIA NAT package family to version ChangesNAT installation
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 `@Dockerfile.agentic-base`:
- Around line 110-113: Align all NAT references to version 1.8.0: update the
installation pin in Dockerfile.agentic-base, the requirement in
tests/agentic-use/requirements-nat.txt, and the Dockerfile example and expected
version assertions in tests/agentic-use/tests/test_agentic_base_dockerfile.py;
preserve the existing installation structure and update every affected static
expectation consistently.
In `@tests/agentic-use/tests/test_agentic_base_dockerfile.py`:
- Around line 31-33: Update the dependency assertion covering nvidia-nat-atif,
nvidia-nat-eval, and nvidia-nat-mcp to validate complete requirement tokens
rather than accepting matching substrings. Require the exact ==1.7.0 pin so
versions such as ==1.7.0.post1 do not pass.
🪄 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: CHILL
Plan: Enterprise
Run ID: d71576ee-2455-43c9-bb66-e5e9e93aa524
📒 Files selected for processing (3)
Dockerfile.agentic-basetests/agentic-use/requirements-nat.txttests/agentic-use/tests/test_agentic_base_dockerfile.py
|
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. |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/agentic-use/tests/test_agentic_base_dockerfile.py (1)
51-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the local prerelease command.
This test validates the package lines but not the documented command in
tests/agentic-use/requirements-nat.txtLines 31-36. A future edit could remove--prerelease=allowwhile the test still passes. Assert the option and documented package tokens.Suggested assertion
def test_local_nat_requirements_pin_package_family() -> None: + local_text = REQUIREMENTS_NAT.read_text() requirements = { line.strip() - for line in REQUIREMENTS_NAT.read_text().splitlines() + for line in local_text.splitlines() if line.strip() and not line.startswith("#") } + assert "uv pip install --prerelease=allow" in local_text assert requirements == EXPECTED_LOCAL_NAT_REQUIREMENTS🤖 Prompt for 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. In `@tests/agentic-use/tests/test_agentic_base_dockerfile.py` around lines 51 - 58, Update test_local_nat_requirements_pin_package_family to also read and validate the documented local prerelease command in REQUIREMENTS_NAT, asserting that it retains --prerelease=allow and the expected package tokens while preserving the existing package-line equality check.
🤖 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.
Nitpick comments:
In `@tests/agentic-use/tests/test_agentic_base_dockerfile.py`:
- Around line 51-58: Update test_local_nat_requirements_pin_package_family to
also read and validate the documented local prerelease command in
REQUIREMENTS_NAT, asserting that it retains --prerelease=allow and the expected
package tokens while preserving the existing package-line equality check.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 12b8055f-471b-48a7-8075-fbcacc7e8b2a
📒 Files selected for processing (3)
Dockerfile.agentic-basetests/agentic-use/requirements-nat.txttests/agentic-use/tests/test_agentic_base_dockerfile.py
3b80686 to
4555845
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. |
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
4555845 to
1456e3d
Compare
Summary
--prerelease=allowto the pinned NAT package-family install inDockerfile.agentic-base.NVBug
Validation
uv run --frozen pytest tests/agentic-use/tests/test_agentic_base_dockerfile.py tests/agentic-use/tests/test_nat_version_consistency.py -q: 3 passeduv run --frozen ruff check tests/agentic-use/tests/test_agentic_base_dockerfile.pyuv run --frozen ty check tests/agentic-use/tests/test_agentic_base_dockerfile.py0.9.14dry-run resolver with the Dockerfile NAT requirements and--prerelease=allow: resolved successfullyNote
docker build -f Dockerfile.agentic-base -t nmp-agentic-base:latest .was attempted, but this environment could not reachkeyserver.ubuntu.com/ppa.launchpadcontent.net; the build failed before the NAT install layer withcurl: (35) Recv failure: Connection reset by peer.Summary by CodeRabbit
Bug Fixes
Tests
Review follow-up
3b80686ab(fix: align agentic NAT dependency pins).4 passed; targeted Ruff and ty checks passed;uv pip install --dry-run --prerelease=allow "nvidia-nat[mcp,eval,langchain]==1.8.0" nvidia-nat-atif==1.8.0 nvidia-nat-eval==1.8.0 nvidia-nat-mcp==1.8.0resolved successfully.uv run pre-commit run -ais blocked by the samehelm-docshook fetch HTTP 503 noted on the related PR.Latest push
45558453eto rerun CI after Actions instability. DCO audit passed.