chore: add nvswitch nvos info switch detail page#3417
Conversation
Signed-off-by: Roopesh Tamma <rtamma@nvidia.com>
Summary by CodeRabbit
WalkthroughSwitch lookup paths now preserve NVOS IP and MAC values independently, including partial data, while leaving ChangesSwitch NVOS details
Estimated code review effort: 2 (Simple) | ~15 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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
🤖 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 `@crates/api-web/templates/switch_detail.html`:
- Around line 121-142: The upstream switch detail mapping in the SwitchNvosInfo
creation flow must honor the template’s optional-field contract: preserve IP and
MAC independently when available and populate port from the source data instead
of always setting it to None. Update the relevant mapping in the switch handler
while keeping the existing nvos_info rendering structure unchanged.
🪄 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: 77f6b4ea-2c72-4519-b2b3-a55895a43b6c
📒 Files selected for processing (2)
crates/api-web/src/switch.rscrates/api-web/templates/switch_detail.html
| <table class="detailsview"> | ||
| {% if let Some(nvos_info) = nvos_info %} | ||
| {% if let Some(ip) = nvos_info.ip %} | ||
| <tr> | ||
| <th>NVOS IP</th> | ||
| <td>{{ ip }}</td> | ||
| </tr> | ||
| {% endif %} | ||
| {% if let Some(mac) = nvos_info.mac %} | ||
| <tr> | ||
| <th>NVOS MAC</th> | ||
| <td>{{ mac }}</td> | ||
| </tr> | ||
| {% endif %} | ||
| {% if let Some(port) = nvos_info.port %} | ||
| <tr> | ||
| <th>NVOS Port</th> | ||
| <td>{{ port }}</td> | ||
| </tr> | ||
| {% endif %} | ||
| {% endif %} | ||
| </table> |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Align the producer with the optional-field rendering contract.
In the supplied crates/api-core/src/handlers/switch.rs flow, SwitchNvosInfo is created only when both IP and MAC are present, and port is always set to None. Consequently, this template cannot render a standalone available IP/MAC value, and the NVOS Port row at Line [135] through Line [140] is unreachable. Update the upstream mapping to preserve independently available fields and populate the port, or adjust the feature scope.
🤖 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 `@crates/api-web/templates/switch_detail.html` around lines 121 - 142, The
upstream switch detail mapping in the SwitchNvosInfo creation flow must honor
the template’s optional-field contract: preserve IP and MAC independently when
available and populate port from the source data instead of always setting it to
None. Update the relevant mapping in the switch handler while keeping the
existing nvos_info rendering structure unchanged.
Source: Path instructions
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
Signed-off-by: Roopesh Tamma <rtamma@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/api-core/src/handlers/switch.rs (1)
555-606: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer a table-driven helper test.
These
Option-returning helper cases can be consolidated withvalue_scenarios!orcheck_values, keeping missing, partial, and port invariants together.As per coding guidelines, tests for total operations should prefer table-driven
value_scenarios!orcheck_values.🤖 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 `@crates/api-core/src/handlers/switch.rs` around lines 555 - 606, Consolidate the three tests in switch_nvos_info_tests into one table-driven test using the project’s value_scenarios! or check_values helper. Keep cases covering missing values, IP-only, MAC-only, and both values, and assert the corresponding Option fields plus the invariant that port remains unset.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 `@crates/api-core/src/handlers/switch.rs`:
- Around line 555-606: Consolidate the three tests in switch_nvos_info_tests
into one table-driven test using the project’s value_scenarios! or check_values
helper. Keep cases covering missing values, IP-only, MAC-only, and both values,
and assert the corresponding Option fields plus the invariant that port remains
unset.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 342bfaf3-814f-419d-b3b1-77c1d682b477
📒 Files selected for processing (3)
crates/api-core/src/handlers/switch.rscrates/api-core/src/tests/switch_find.rscrates/api-web/templates/switch_detail.html
💤 Files with no reviewable changes (1)
- crates/api-web/templates/switch_detail.html
1 participant
Add NVOS info (IP, MAC, port) to the Interfaces section of switch detail page
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes