Conversation
Signed-off-by: Wenqi Li <wenqil@nvidia.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds two JSON release manifests under releases/4.0.0 ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
releases/4.0.0/artifacts-cu12.json (1)
26-29:grpc-health-probev0.4.38 is pinned across all entries — a newer version is availableThe latest grpc-health-probe release is
v0.4.39(and further releases exist up to v0.4.45), while all entries pinv0.4.38. Pinning is generally safe and v0.4.38 is a valid, confirmed release, but consider evaluating whether a newer version should be adopted for the 4.0.0 manifest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@releases/4.0.0/artifacts-cu12.json` around lines 26 - 29, The "health-probes" entries currently pin grpc-health-probe to v0.4.38; update the two URLs under the "health-probes" key to point to the newer release (e.g., change the version segment from v0.4.38 to v0.4.39 in the linux/amd64 and linux/arm64 URLs), then run any release validation you use (download/sha/checks) to confirm the new binaries; alternatively, if you intend to adopt a different newer patch (v0.4.40+), replace v0.4.38 with that chosen tag consistently in both URLs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@releases/4.0.0/artifacts-cu12.json`:
- Around line 10-24: The build-images entries ("build-images" -> "igpu", "dgpu",
"cpu") currently reference the staging registry prefix
"nvcr.io/nvstaging/holoscan/holoscan:v4.0.0.0-cuda12-*" and must be updated to
the production registry paths; locate the image strings under the keys igpu,
dgpu, and cpu and replace the "nvcr.io/nvstaging/..." prefixes with the correct
production registry/image names for v4.0.0.0-cuda12 (ensuring each image tag
stays the same, e.g., replace
nvcr.io/nvstaging/holoscan/holoscan:v4.0.0.0-cuda12-igpu with the corresponding
production registry path).
In `@releases/4.0.0/artifacts.json`:
- Around line 9-18: The manifest's build-images entries use the staging registry
host "nvcr.io/nvstaging" (see keys "build-images" -> "dgpu"/"cpu" image
strings), which will be unreachable to most users; replace those staging image
references with the production registry namespace (e.g., change
"nvcr.io/nvstaging/holoscan/holoscan:v4.0.0.0-cuda13" to the agreed production
host such as "nvcr.io/nvidia/holoscan/holoscan:v4.0.0.0-cuda13" or
"nvcr.io/nvidia/clara-holoscan/holoscan:v4.0.0.0-cuda13") and ensure the
corresponding images are promoted to that production registry before this
manifest is merged.
---
Nitpick comments:
In `@releases/4.0.0/artifacts-cu12.json`:
- Around line 26-29: The "health-probes" entries currently pin grpc-health-probe
to v0.4.38; update the two URLs under the "health-probes" key to point to the
newer release (e.g., change the version segment from v0.4.38 to v0.4.39 in the
linux/amd64 and linux/arm64 URLs), then run any release validation you use
(download/sha/checks) to confirm the new binaries; alternatively, if you intend
to adopt a different newer patch (v0.4.40+), replace v0.4.38 with that chosen
tag consistently in both URLs.
Signed-off-by: JC Fillion-Robin <jcfr@nvidia.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
releases/4.0.0/artifacts.json (1)
10-17:⚠️ Potential issue | 🟠 MajorSwitch 4.0.0 build images from staging to production registry before merge.
The
4.0.0manifest still pointsbuild-imagestonvcr.io/nvstaging/...(dgpuandcpu). That will fail for users without staging access when this manifest is consumed.#!/bin/bash # Read-only check: find staging registry usage across release manifests. fd 'artifacts.*\.json$' releases -t f -x sh -c ' echo "== $1 =="; rg -n "nvcr\.io/nvstaging/" "$1" || true ' sh {}Please replace with the agreed production namespace/tag set in this file (and mirror it in
artifacts-cu12.jsonif it also contains staging refs).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@releases/4.0.0/artifacts.json` around lines 10 - 17, The artifacts.json still references the staging registry image tags for dgpu and cpu (e.g., values under "dgpu" -> "x64-workstation" and "sbsa", and "cpu" -> "x64-workstation") which will fail for users without staging access; update those image strings from nvcr.io/nvstaging/... to the agreed production namespace and tag (the production nvcr.io/... holoscan:v4.0.0.1-cuda13 equivalent) and ensure the same replacements are applied in the corresponding artifacts-cu12.json so both manifests point to production images.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@releases/4.0.0/artifacts.json`:
- Around line 10-17: The artifacts.json still references the staging registry
image tags for dgpu and cpu (e.g., values under "dgpu" -> "x64-workstation" and
"sbsa", and "cpu" -> "x64-workstation") which will fail for users without
staging access; update those image strings from nvcr.io/nvstaging/... to the
agreed production namespace and tag (the production nvcr.io/...
holoscan:v4.0.0.1-cuda13 equivalent) and ensure the same replacements are
applied in the corresponding artifacts-cu12.json so both manifests point to
production images.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
releases/4.0.0/artifacts-cu12.jsonreleases/4.0.0/artifacts.json
✅ Files skipped from review due to trivial changes (1)
- releases/4.0.0/artifacts-cu12.json
Signed-off-by: JC Fillion-Robin <jcfr@nvidia.com>
Summary by CodeRabbit