fix: Add URL in SoftwareInventory in UI/cli.#3781
Merged
Merged
Conversation
Contributor
|
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:
Summary by CodeRabbit
WalkthroughThe change aligns DPF service-version lookup naming, derives service URLs from image or Helm metadata, computes the DPU name from BMC identifiers, propagates URLs into machine inventory, and broadens HBN component matching in DPU version views. ChangesDPF service metadata and inventory
Broaden HBN version extraction
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant InventoryHandler
participant DpfSdk
participant DPUDeployment
InventoryHandler->>InventoryHandler: Load managed host snapshot
InventoryHandler->>InventoryHandler: Derive dpu_name from DPU and host dpf_id values
InventoryHandler->>DpfSdk: get_service_versions_for_dpu(dpu_name)
DpfSdk->>DPUDeployment: Read image or Helm metadata
DPUDeployment-->>DpfSdk: Return service metadata
DpfSdk-->>InventoryHandler: Return service versions with URLs
InventoryHandler->>InventoryHandler: Populate software component URLs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
abvarshney-nv
force-pushed
the
update_url_inventory
branch
from
July 21, 2026 17:57
d424336 to
4073df8
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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-core/src/handlers/dpu.rs`:
- Around line 841-843: Normalize the CarbideError messages in the DPU/DPF error
handling around the visible DPF SDK message and the related messages at the
referenced locations: start each message with lowercase wording, including
acronyms such as DPF/DPU where needed, preserve the original meaning, and omit
trailing periods.
🪄 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: 7ff033ee-f84f-4de5-8f2a-8bb1898a27c2
📒 Files selected for processing (4)
crates/api-core/src/handlers/dpu.rscrates/dpf/src/sdk.rscrates/dpf/src/types.rscrates/machine-controller/src/dpf.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/dpf/src/types.rs
- crates/dpf/src/sdk.rs
abvarshney-nv
force-pushed
the
update_url_inventory
branch
2 times, most recently
from
July 21, 2026 18:23
fd0962e to
c657b7f
Compare
abvarshney-nv
force-pushed
the
update_url_inventory
branch
from
July 21, 2026 18:38
c657b7f to
58f8485
Compare
wminckler
approved these changes
Jul 21, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/admin-cli/src/dpu/versions/cmd.rs (1)
104-104: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the broadened HBN matching in both conversion paths.
The supplied
doca-hbnfixture was already supported, so it cannot detect regressions in the new substring behavior. Add aligned table-driven cases for a non-legacy HBN-containing name, both legacy names, and a non-match.
crates/admin-cli/src/dpu/versions/cmd.rs#L104-L104: verify the CLI conversion extracts the expected version.crates/api-web/src/dpu_versions.rs#L109-L109: verify the web conversion produces the same result.🤖 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/admin-cli/src/dpu/versions/cmd.rs` at line 104, Add aligned table-driven tests covering a non-legacy HBN-containing name, both legacy names, and a non-match, asserting the expected version for each case. Update tests near the CLI conversion logic in crates/admin-cli/src/dpu/versions/cmd.rs:104-104 and the web conversion logic in crates/api-web/src/dpu_versions.rs:109-109; both sites require equivalent coverage and results.Sources: Coding guidelines, Learnings
🤖 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/dpf/src/types.rs`:
- Around line 480-482: Update the documentation for the `name` field in the
relevant type to describe the third fallback: when both the image basename and
Helm chart name are empty, it uses the DPUDeployment service name assigned by
`get_service_versions_for_dpu`. Preserve the existing image and Helm fallback
descriptions.
---
Nitpick comments:
In `@crates/admin-cli/src/dpu/versions/cmd.rs`:
- Line 104: Add aligned table-driven tests covering a non-legacy HBN-containing
name, both legacy names, and a non-match, asserting the expected version for
each case. Update tests near the CLI conversion logic in
crates/admin-cli/src/dpu/versions/cmd.rs:104-104 and the web conversion logic in
crates/api-web/src/dpu_versions.rs:109-109; both sites require equivalent
coverage and results.
🪄 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: 692a6a70-ae36-4453-a415-d16f7639f10d
📒 Files selected for processing (6)
crates/admin-cli/src/dpu/versions/cmd.rscrates/api-core/src/handlers/dpu.rscrates/api-web/src/dpu_versions.rscrates/dpf/src/sdk.rscrates/dpf/src/types.rscrates/machine-controller/src/dpf.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/machine-controller/src/dpf.rs
- crates/api-core/src/handlers/dpu.rs
- crates/dpf/src/sdk.rs
abvarshney-nv
enabled auto-merge (squash)
July 21, 2026 18:43
10 tasks
10 tasks
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
2 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.
Add URL in SoftwareInventory in UI/cli.
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes