feat(nemo-agents): update fabric dep to use published wheel and fabric validation helpers#761
Conversation
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
|
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:
📝 WalkthroughWalkthroughNeMo Fabric dependencies were added, Fabric translation and platform validation were updated, tests now patch imported SDK objects, and generated dependency and security metadata were refreshed. ChangesNeMo Fabric integration
Sequence Diagram(s)sequenceDiagram
participant PlatformConfig
participant translate_agent_config
participant Fabric
participant validate_fabric_config
PlatformConfig->>translate_agent_config: coerce and translate AgentConfig
translate_agent_config->>Fabric: construct FabricConfig
validate_fabric_config->>Fabric: run plan and doctor
validate_fabric_config-->>PlatformConfig: return validation result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
plugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.py (1)
30-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType
fabric_configasFabricConfig.Translation guarantees this type; retaining
Anyweakens the new result contract.Proposed change
-from nemo_fabric import Fabric, FabricConfigError +from nemo_fabric import Fabric, FabricConfig, FabricConfigError class PlatformFabricValidationResult: agent_config: AgentConfig - fabric_config: Any + fabric_config: FabricConfigAs per coding guidelines, prefer concrete type hints and import those types normally when possible.
🤖 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 `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.py` around lines 30 - 37, Update the PlatformFabricValidationResult dataclass so its fabric_config field uses the concrete FabricConfig type instead of Any, and add the normal import for FabricConfig. Leave the other result fields unchanged.Source: Coding guidelines
plugins/nemo-agents/tests/unit/test_fabric_translator.py (1)
42-53: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd one real Fabric-model translation smoke test.
Both fixtures accept arbitrary keywords and therefore cannot detect SDK schema drift.
plugins/nemo-agents/tests/unit/test_fabric_translator.py#L42-L53: leave at least one translation test unpatched.plugins/nemo-agents/tests/unit/test_fabric_validation.py#L105-L116: reuse that real-model coverage rather than relying exclusively on the duplicate fake.🤖 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 `@plugins/nemo-agents/tests/unit/test_fabric_translator.py` around lines 42 - 53, The fake fabric_models fixture masks SDK schema drift because its substitute models accept arbitrary keywords. In plugins/nemo-agents/tests/unit/test_fabric_translator.py:42-53, leave at least one translation test using the real Fabric models instead of this patched fixture. In plugins/nemo-agents/tests/unit/test_fabric_validation.py:105-116, reuse that real-model translation coverage and remove reliance on the duplicate fake fixture where applicable.plugins/nemo-agents/tests/unit/test_fabric_validation.py (1)
45-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the string return annotation with
Self.Import
Selffromtypingortyping_extensions, according to the declared Python target.As per coding guidelines, prefer concrete type hints over string-based hints and import them normally.
🤖 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 `@plugins/nemo-agents/tests/unit/test_fabric_validation.py` around lines 45 - 51, Update the enable_relay method’s return annotation to use the imported Self type instead of a string-based "_FakeFabricConfig" annotation, sourcing Self from typing or typing_extensions according to the project’s Python target.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.
Nitpick comments:
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.py`:
- Around line 30-37: Update the PlatformFabricValidationResult dataclass so its
fabric_config field uses the concrete FabricConfig type instead of Any, and add
the normal import for FabricConfig. Leave the other result fields unchanged.
In `@plugins/nemo-agents/tests/unit/test_fabric_translator.py`:
- Around line 42-53: The fake fabric_models fixture masks SDK schema drift
because its substitute models accept arbitrary keywords. In
plugins/nemo-agents/tests/unit/test_fabric_translator.py:42-53, leave at least
one translation test using the real Fabric models instead of this patched
fixture. In plugins/nemo-agents/tests/unit/test_fabric_validation.py:105-116,
reuse that real-model translation coverage and remove reliance on the duplicate
fake fixture where applicable.
In `@plugins/nemo-agents/tests/unit/test_fabric_validation.py`:
- Around line 45-51: Update the enable_relay method’s return annotation to use
the imported Self type instead of a string-based "_FakeFabricConfig" annotation,
sourcing Self from typing or typing_extensions according to the project’s Python
target.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f3ff3323-4e89-4560-bc8d-1e101bcd8e7c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
packages/nemo_platform/pyproject.tomlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.pyplugins/nemo-agents/tests/unit/test_fabric_translator.pyplugins/nemo-agents/tests/unit/test_fabric_validation.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
|
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Summary
This PR implements AIRCORE-897 by wiring the NeMo Agents Fabric config translation path into Fabric SDK planning and diagnostics.
The validation flow established here is:
This keeps the Platform-owned
nemo-agents-spec-v1config as the persisted contract while using Fabric SDK APIs to validate the translated execution config before later register/deploy/invoke wiring.Changes
nemo-agents-plugin.validate_platform_agent_config(...)as the Platform-facing validation helper.validate_fabric_config(...)as the lower-level FabricConfig validation primitive.plan()off the event loop.doctor()with a validation timeout.Notes
nemo-fabricis installed by default for SDK config translation, planning, and diagnostics.nemo-fabric-runtimeis intentionally not installed by default because its native install path currently requiresnvidia-smi, which is not available during service image builds.nemo-relaypin conflict.Out Of Scope
Those are covered by follow-up AIRCORE-895 implementation tickets.
Validation
Passed:
Result:
37 passedSummary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores