feat: display pod labels in task list and detail views#110
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pingxin403 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 |
|
Welcome @pingxin403! It looks like this is your first PR to Project-HAMi/HAMi-WebUI 🎉 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds container labels to backend responses and shows them in the web task admin views with matching English and Chinese labels. ChangesContainer Labels
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ContainerService
participant PodRepo
participant ContainerReply
Client->>ContainerService: GetContainer / GetAllContainers
ContainerService->>PodRepo: fetch container(s)
PodRepo-->>ContainerService: Container with Labels map
ContainerService->>ContainerReply: assign Labels from Container.Labels
ContainerService-->>Client: reply with 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
packages/web/projects/vgpu/views/task/admin/index.vueParsing error: Cannot find module '
Make sure that all the Babel plugins and presets you are using 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
🧹 Nitpick comments (1)
server/internal/data/pod.go (1)
142-156: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider copying
pod.Labelsinstead of sharing the reference.
pod.Labelsis assigned by reference to everybiz.Containerbuilt from this pod, andpodtypically originates from an informer/lister cache. Per client-go convention, objects retrieved from Listers/Informers are cache pointers that must not be mutated without copying first, otherwise the cache is corrupted. Sharing this map across multiple containers (and potentially with the cached object) is safe today only because nothing currently mutates it downstream — if that changes, it becomes a shared-mutation hazard affecting the informer cache and other containers of the same pod.🛡️ Proposed defensive copy
+ labels := make(map[string]string, len(pod.Labels)) + for k, v := range pod.Labels { + labels[k] = v + } c := &biz.Container{ Name: ctr.Name, UUID: ctrIdMaps[ctr.Name], ContainerIdx: i, NodeName: pod.Spec.NodeName, PodName: pod.Name, PodUID: string(pod.UID), Image: ctr.Image, Status: containerStat[ctr.Name], NodeUID: r.GetNodeUUID(pod), Namespace: pod.Namespace, CreateTime: r.GetCreateTime(pod), ContainerDevices: containerDevices, - Labels: pod.Labels, + Labels: labels, }🤖 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/data/pod.go` around lines 142 - 156, The Container construction in pod data handling is reusing pod.Labels by reference, which can create shared-mutation risk with informer/lister cache objects. In the code that builds biz.Container inside the pod processing logic, copy the labels map defensively before assigning it to Labels so each container gets its own map rather than sharing pod.Labels directly.
🤖 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 `@packages/web/projects/vgpu/views/task/admin/index.vue`:
- Around line 193-197: Add a stable key to the `<t-tag>` elements rendered by
`visible.map(...)` in the task admin view. The list currently omits `key`, so
update the mapped tag in the same render block to use the item’s identifier
(`k`) consistently, matching the pattern used in `Detail.vue`, so Vue can track
each tag correctly across re-renders.
---
Nitpick comments:
In `@server/internal/data/pod.go`:
- Around line 142-156: The Container construction in pod data handling is
reusing pod.Labels by reference, which can create shared-mutation risk with
informer/lister cache objects. In the code that builds biz.Container inside the
pod processing logic, copy the labels map defensively before assigning it to
Labels so each container gets its own map rather than sharing pod.Labels
directly.
🪄 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: 88dd55e3-144c-448c-8c1c-2dcd24a3e3c0
📒 Files selected for processing (9)
packages/web/projects/vgpu/views/task/admin/Detail.vuepackages/web/projects/vgpu/views/task/admin/index.vuepackages/web/src/locales/en.jspackages/web/src/locales/zh.jsserver/api/v1/container.protoserver/internal/biz/pod.goserver/internal/data/pod.goserver/internal/service/container.goserver/internal/service/container_test.go
Add Labels field to ContainerReply proto, biz model, and data layer. Populate labels from pod.Labels in fetchContainerInfo. Add Tags column to task list (first 2 labels + N overflow). Add Labels row to task detail basic info. Closes Project-HAMi#85 Signed-off-by: pingxin403 <pingxin403@users.noreply.github.com>
pingxin403
force-pushed
the
feat/labels-exported-pod
branch
from
6a28db6 to
fc09ba9
Compare
Address CodeRabbit review comments: - Copy pod.Labels map to avoid informer cache shared-mutation risk - Add missing :key to t-tag in task list Labels column Signed-off-by: pingxin403 <52284317+pingxin403@users.noreply.github.com>
1 participant
Description
Add Pod labels display to HAMi-WebUI task management pages.
Backend changes
Labels map[string]stringtobiz.Containerstructmap<string, string> labels = 20toContainerReplyprotopod.LabelsinfetchContainerInfoGetAllContainers/GetContainer)Frontend changes
Closes #85
Summary by CodeRabbit
--placeholder.