use exported_namespace/pod labels for NVIDIA task metrics queries - #114
use exported_namespace/pod labels for NVIDIA task metrics queries#114Shenhan11 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThe change expands GPU, node, and container resource data across backend APIs and services, adds node and card administration filters and usage displays, adds task compute configuration details, updates localization, and corrects NVIDIA metric label selectors. ChangesResource contracts and data mapping
Backend resource services
NVIDIA metric selectors
vGPU administration views
Localization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
Signed-off-by: Shenhan11 <han.shen@dynamia.ai>
4346d10 to
88c8e6d
Compare
hami-robot
Bot
added
dco-signoff: yes
dco-signoff: no
size/XL
and removed
dco-signoff: no
dco-signoff: yes
size/XS
labels
063f6de to
88c8e6d
Compare
hami-robot
Bot
added
dco-signoff: yes
size/XS
and removed
dco-signoff: no
size/XL
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/internal/service/node.go (1)
160-171: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve metric availability information.
This helper currently returns only a value map, so callers cannot distinguish “no sample” from “query failed.” Return an error or validity status alongside the map and handle it explicitly at the API boundary.
🤖 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 `@server/internal/service/node.go` around lines 160 - 171, The queryNodeGauge helper currently hides query failures by returning only an empty map. Update NodeService.queryNodeGauge to return the gauge map together with an error or explicit validity status, propagate the QueryInstant failure, and update its API-boundary callers to handle that result distinctly from a successful query with no samples.
🧹 Nitpick comments (4)
packages/web/projects/vgpu/views/card/admin/index.vue (1)
204-240: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
renderPercentPairimplementation.Nearly identical to the
renderPercentPairadded inpackages/web/projects/vgpu/views/node/admin/index.vue. Consider extracting to a shared composable/util. See consolidated comment.🤖 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 `@packages/web/projects/vgpu/views/card/admin/index.vue` around lines 204 - 240, Extract the duplicated renderPercentPair logic from the card admin view and the node admin view into a shared composable or utility, including its clamping, formatting, progress rendering, and resource-color behavior. Replace both local implementations with the shared symbol while preserving their existing output.packages/web/projects/vgpu/views/node/admin/index.vue (2)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract duplicated
renderPercentPair/getNodeRoleTexthelpers into a shared composable. This PR introduces the samerenderPercentPairimplementation in two files and the samegetNodeRoleTextimplementation in two other files; all four copies are verbatim duplicates with no shared source.
packages/web/projects/vgpu/views/node/admin/index.vue#L128-172: extract bothgetNodeRoleTextandrenderPercentPairinto a shared composable/util (e.g.~/vgpu/hooks/useResourceDisplay.js) and import here.packages/web/projects/vgpu/views/card/admin/index.vue#L204-240: replace the localrenderPercentPairwith the shared implementation.packages/web/projects/vgpu/views/node/admin/Detail.vue#L362-377: replace the localgetNodeRoleTextwith the shared implementation.🤖 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 `@packages/web/projects/vgpu/views/node/admin/index.vue` at line 1, Extract the duplicated getNodeRoleText and renderPercentPair helpers from the node and card admin views into one shared composable or utility, such as useResourceDisplay. Export both helpers there, then import and use them in node/admin/index.vue, card/admin/index.vue, and node/admin/Detail.vue, removing the local duplicate implementations while preserving their existing behavior.
128-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
getNodeRoleText/renderPercentPairhelpers.
renderPercentPairduplicates the one incard/admin/index.vue;getNodeRoleTextduplicates the one inDetail.vue. Consider extracting both into shared composables. See consolidated comment.🤖 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 `@packages/web/projects/vgpu/views/node/admin/index.vue` around lines 128 - 172, Extract the duplicated getNodeRoleText and renderPercentPair helpers from the node admin views into shared composables, then update index.vue and the corresponding card/admin/index.vue and Detail.vue usages to import and reuse them. Preserve the existing translations, percentage clamping, formatting, progress rendering, and resource-color behavior.packages/web/projects/vgpu/views/node/admin/Detail.vue (1)
362-377: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated
getNodeRoleTexthelper.Identical implementation to the one added in
node/admin/index.vue. See consolidated comment.🤖 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 `@packages/web/projects/vgpu/views/node/admin/Detail.vue` around lines 362 - 377, Remove the duplicated getNodeRoleText helper from Detail.vue and reuse the existing shared or index.vue implementation identified in the consolidated comment. Update the node role column renderer in detailColumns to call that single implementation while preserving the current translations and unknown-role fallback.
🤖 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 `@server/internal/service/node.go`:
- Around line 55-72: Extract the CoreUsage and MemoryUsage enrichment currently
performed in the GetAllNodes loop into a reusable helper around buildNodeReply.
Apply that helper in both GetAllNodes and GetNode, using the corresponding
coreUsageByNode and memUsageByNode values so both endpoints populate usage
metrics consistently.
- Around line 55-72: Update queryNodeGauge and the node-reply construction
around buildNodeReply so QueryInstant failures preserve metric unavailability
instead of leaving protobuf utilization fields at zero. Propagate query
success/validity alongside each gauge result, and only assign CoreUsage and
MemoryUsage when the corresponding metric query succeeded; otherwise expose the
established unavailable representation to consumers.
---
Outside diff comments:
In `@server/internal/service/node.go`:
- Around line 160-171: The queryNodeGauge helper currently hides query failures
by returning only an empty map. Update NodeService.queryNodeGauge to return the
gauge map together with an error or explicit validity status, propagate the
QueryInstant failure, and update its API-boundary callers to handle that result
distinctly from a successful query with no samples.
---
Nitpick comments:
In `@packages/web/projects/vgpu/views/card/admin/index.vue`:
- Around line 204-240: Extract the duplicated renderPercentPair logic from the
card admin view and the node admin view into a shared composable or utility,
including its clamping, formatting, progress rendering, and resource-color
behavior. Replace both local implementations with the shared symbol while
preserving their existing output.
In `@packages/web/projects/vgpu/views/node/admin/Detail.vue`:
- Around line 362-377: Remove the duplicated getNodeRoleText helper from
Detail.vue and reuse the existing shared or index.vue implementation identified
in the consolidated comment. Update the node role column renderer in
detailColumns to call that single implementation while preserving the current
translations and unknown-role fallback.
In `@packages/web/projects/vgpu/views/node/admin/index.vue`:
- Line 1: Extract the duplicated getNodeRoleText and renderPercentPair helpers
from the node and card admin views into one shared composable or utility, such
as useResourceDisplay. Export both helpers there, then import and use them in
node/admin/index.vue, card/admin/index.vue, and node/admin/Detail.vue, removing
the local duplicate implementations while preserving their existing behavior.
- Around line 128-172: Extract the duplicated getNodeRoleText and
renderPercentPair helpers from the node admin views into shared composables,
then update index.vue and the corresponding card/admin/index.vue and Detail.vue
usages to import and reuse them. Preserve the existing translations, percentage
clamping, formatting, progress rendering, and resource-color behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d2440968-f99c-4aab-b4b8-9426f02a3579
📒 Files selected for processing (17)
packages/web/projects/vgpu/views/card/admin/index.vuepackages/web/projects/vgpu/views/node/admin/Detail.vuepackages/web/projects/vgpu/views/node/admin/index.vuepackages/web/projects/vgpu/views/task/admin/index.vuepackages/web/src/locales/en.jspackages/web/src/locales/zh.jsserver/api/v1/card.protoserver/api/v1/container.protoserver/api/v1/node.protoserver/internal/biz/node.goserver/internal/biz/pod.goserver/internal/data/node.goserver/internal/data/pod.goserver/internal/exporter/exporter.goserver/internal/service/card.goserver/internal/service/container.goserver/internal/service/node.go
🚧 Files skipped from review as they are similar to previous changes (1)
- server/internal/exporter/exporter.go
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: FouoF, Shenhan11 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
API & Localization