fix: fix nat crash in nvbug 6336437#417
Open
gabwow wants to merge 2 commits into
Open
Conversation
gabwow
force-pushed
the
nvbug6336437-fix-nat-crash/agabow
branch
from
9aa00e4 to
ad3e694
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAll Changesnvidia-nat Version Pinning and Drift Guard
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (1)
tests/agentic-use/tests/test_nat_version_consistency.py (1)
18-18: 📐 Maintainability & Code Quality | 🔵 TrivialRemove
from __future__ import annotationsto align with concrete type hints.Line 18 conflicts with the repo guideline to prefer concrete type hints. The file uses only concrete annotations (
dict[str, str],None) and has no forward references requiring postponed evaluation.🤖 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_nat_version_consistency.py` at line 18, Remove the `from __future__ import annotations` import statement from the file. Since the test file only uses concrete type hints like `dict[str, str]` and `None` with no forward references, the postponed evaluation feature is unnecessary and conflicts with the repository's preference for concrete type annotations. Simply delete line 18 containing this import.Source: Coding guidelines
🤖 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 `@tests/agentic-use/tests/test_nat_version_consistency.py`:
- Around line 6-8: The docstring in the test file incorrectly states that the
agentic-base image installs the NAT family "unpinned," but this is outdated
since Dockerfile.agentic-base now pins nvidia-nat to 1.7.0. Update the docstring
text starting with "The agentic-base image installs the NAT family unpinned" to
accurately reflect that NAT is currently pinned to version 1.7.0 in the
Dockerfile.
---
Nitpick comments:
In `@tests/agentic-use/tests/test_nat_version_consistency.py`:
- Line 18: Remove the `from __future__ import annotations` import statement from
the file. Since the test file only uses concrete type hints like `dict[str,
str]` and `None` with no forward references, the postponed evaluation feature is
unnecessary and conflicts with the repository's preference for concrete type
annotations. Simply delete line 18 containing this import.
🪄 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: 3dc72581-4675-47ea-b0bb-75dd9fe5a55b
📒 Files selected for processing (2)
Dockerfile.agentic-basetests/agentic-use/tests/test_nat_version_consistency.py
Comment on lines
+6
to
+8
| The agentic-base image installs the NAT family unpinned (see | ||
| Dockerfile.agentic-base / tests/agentic-use/requirements-nat.txt). The CLI/meta | ||
| package historically lagged the plugin packages (e.g. ``nvidia-nat==1.4.3`` vs |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update stale docstring claim about pinning.
Line 6 says NAT is installed “unpinned,” but Dockerfile.agentic-base now pins nvidia-nat* to 1.7.0. Update this text to reflect current behavior.
🤖 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_nat_version_consistency.py` around lines 6 - 8,
The docstring in the test file incorrectly states that the agentic-base image
installs the NAT family "unpinned," but this is outdated since
Dockerfile.agentic-base now pins nvidia-nat to 1.7.0. Update the docstring text
starting with "The agentic-base image installs the NAT family unpinned" to
accurately reflect that NAT is currently pinned to version 1.7.0 in the
Dockerfile.
Signed-off-by: Aaron Gabow <agabow@nvidia.com>
Signed-off-by: Aaron Gabow <agabow@nvidia.com>
gabwow
force-pushed
the
nvbug6336437-fix-nat-crash/agabow
branch
from
ad3e694 to
4dcfb06
Compare
Contributor
|
mckornfield
reviewed
Jun 23, 2026
| # nvidia-nat-core/eval/langchain==1.7.0), which caused ImportErrors at plugin | ||
| # discovery (e.g. register_dataset_loader) and crashed `nat start fastapi`. | ||
| RUN uv pip install --python /app/.venv/bin/python \ | ||
| "nvidia-nat[most]==1.7.0" nvidia-nat-atif==1.7.0 nvidia-nat-eval==1.7.0 nvidia-nat-mcp==1.7.0 && \ |
Contributor
There was a problem hiding this comment.
is there a way this can be enforced in one place, i.e., a text file or smth we copy in? this is good to fix though
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Address https://nvbugspro.nvidia.com/bug/6336437 by pinning all nat to 1.7 add a very small test.
Summary by CodeRabbit
nvidia-nat*package family with consistent version pinning (1.7.0) to avoid installation/version drift.nvidia-nat*versions and fails with a clear report, ensuring alignment stays intact over time.