Skip to content

feat:add dataset/model/block mount count#397

Open
LeiXZ wants to merge 5 commits intoraids-lab:mainfrom
LeiXZ:feat/datasetmountcount
Open

feat:add dataset/model/block mount count#397
LeiXZ wants to merge 5 commits intoraids-lab:mainfrom
LeiXZ:feat/datasetmountcount

Conversation

@LeiXZ
Copy link
Copy Markdown
Contributor

@LeiXZ LeiXZ commented Apr 26, 2026

No description provided.

Copilot AI review requested due to automatic review settings April 26, 2026 14:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

该 PR 旨在为数据集/模型等资源增加“挂载次数(mountCount)”字段,并在前端列表展示与排序;同时在后端作业创建流程中尝试在提交作业后对被挂载的数据集进行计数累加,并为数据表新增 mount_count 列与迁移。

Changes:

  • 前端:扩展 IDataset 增加 mountCount,数据列表新增按挂载次数/创建时间排序及挂载次数展示(含 i18n 文案)。
  • 后端:datasets 表新增 mount_count 字段(model/query/迁移/接口响应同步),并在创建 Volcano 作业时尝试累加挂载次数。
  • 作业创建流程:Jupyter/WebIDE/Custom handler 中从 submitJob 改为直接 client.Create 并手工创建 ingress/forward(该部分与“挂载次数”目标相比属于更大范围的行为改动)。

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
frontend/src/services/api/dataset.ts IDataset 增加 mountCount 字段以承接后端返回
frontend/src/i18n/locales/zhCN/translation.json 增加挂载次数与排序相关中文文案
frontend/src/i18n/locales/enUS/translation.json 增加挂载次数与排序相关英文文案
frontend/src/components/layout/data-list.tsx 列表增加 mountCount 展示与排序字段选择;引入 Tooltip 展示说明
frontend/src/components/file/data-view.tsx 将后端返回的 mountCount 映射到列表项
backend/internal/handler/vcjob/mount_counter.go 新增挂载次数累加逻辑(按去重 datasetID 批量 +1)
backend/internal/handler/vcjob/tensorflow.go 作业创建后调用挂载次数累加
backend/internal/handler/vcjob/pytorch.go 作业创建后调用挂载次数累加
backend/internal/handler/vcjob/jupyter.go 改为直接创建 Job 并手工创建 ingress/forward,同时调用挂载次数累加
backend/internal/handler/vcjob/webide.go 同上(WebIDE),并引入 randomSuffix 用于命名 ingress
backend/internal/handler/vcjob/custom.go 同上(Custom training),并在创建后调用挂载次数累加
backend/internal/handler/dataset.go API 响应结构增加 mountCount 并在转换函数中赋值
backend/dao/model/dataset.go Dataset 模型增加 MountCount 字段并映射到 mount_count
backend/dao/query/datasets.gen.go gorm-gen query 字段映射增加 mount_count
backend/cmd/gorm-gen/models/migrate.go 新增迁移:为 datasets 增加/回滚 mount_count

Comment on lines +338 to +342
<TooltipProvider delayDuration={100}>
<Tooltip>
<TooltipTrigger asChild>
<div className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium">
<BarChart3Icon className="size-4" />
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】这里包了一层 TooltipProvider delayDuration={100} 期望控制延迟,但当前 @/components/ui/tooltipTooltip 组件内部已经固定包了一个 TooltipProvider(delayDuration=0)(嵌套 provider 会导致外层 delay 配置不生效),同时在列表里为每个 item 创建 provider 也会增加不必要的开销。建议:要么移除这里的外层 TooltipProvider,要么调整 Tooltip 组件实现为不在内部创建 Provider/允许透传 delayDuration,并在页面更高层统一放置 Provider。

Copilot uses AI. Check for mistakes.
Comment on lines +177 to +181
if err = mgr.client.Create(c, &job); err != nil {
resputil.Error(c, err.Error(), resputil.NotSpecified)
return
}
if err := increaseDatasetMountCount(c, req.VolumeMounts); err != nil {
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】这里从 mgr.submitJob(...) 改成了直接 mgr.client.Create(...),会绕过 submitJob 中的 prequeue / quota 检查以及 vcjobservice.ActivateJob 的资源创建与失败回滚逻辑(ActivateJob 在创建 access resources 失败时会自动删除 job)。建议恢复使用 mgr.submitJob(或直接调用 vcjobservice.ActivateJob 并保留其 cleanup 语义),避免出现作业不该立即创建却被创建、以及 ingress/forward 创建失败后 job 遗留的情况;同类改动在 webide/custom 也有,请一并核对。

Copilot uses AI. Check for mistakes.
Comment on lines +205 to +207
if err := increaseDatasetMountCount(c, req.VolumeMounts); err != nil {
klog.Warningf("failed to increase dataset mount count for job %s: %v", jobName, err)
}
Copy link

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】submitJob 在命中 prequeue 条件时会“返回 nil 但并不实际 ActivateJob/创建 K8s Job”(仅落库 prequeue record)。当前在 submitJob 之后立刻 increaseDatasetMountCount(...) 会导致 mountCount 在作业尚未真正创建/挂载前就被累计,数据口径不准。建议把计数逻辑挪到真正激活作业的路径(例如在 ActivateJob 前后增加一个“是否已激活”的返回值,或把挂载的 datasetIDs 写入 job annotations/record 并在激活时统一递增);同类调用在 pytorch 等 handler 也存在,请统一处理。

Suggested change
if err := increaseDatasetMountCount(c, req.VolumeMounts); err != nil {
klog.Warningf("failed to increase dataset mount count for job %s: %v", jobName, err)
}
// NOTE:
// `submitJob` may return nil when the job only enters prequeue and has not been
// actually activated/created in Kubernetes yet. Dataset mount count must be
// increased only on the real activation path to keep metrics accurate.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Comment on lines +42 to +46
func parseMountedDatasetIDs(raw string) ([]uint, error) {
var ids []uint
if err := json.Unmarshal([]byte(raw), &ids); err != nil {
return nil, fmt.Errorf("invalid mounted dataset ids annotation: %w", err)
}
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【优化建议】新增的 parseMountedDatasetIDs / increaseDatasetMountCount 包含去重、过滤 0、JSON 解析错误处理等逻辑,但当前 vcjob 包已有 runtime_test.go 的单测体系,这部分没有对应单测覆盖。建议至少补充 parseMountedDatasetIDs 的单元测试(非法 JSON、包含 0、重复 ID、乱序输入等场景),以避免后续改动引入计数偏差。

Copilot uses AI. Check for mistakes.
Comment on lines +342 to +345
<TooltipProvider delayDuration={100}>
<Tooltip>
<TooltipTrigger asChild>
<div className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium">
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【优化建议】这里包了一层 TooltipProvider 并设置 delayDuration=100,但当前 Tooltip 组件内部会固定再包一层 TooltipProvider(默认 delayDuration=0),导致这里的 delayDuration 实际不会生效。建议改为:要么在 Tooltip 组件层面支持透传 delayDuration/不内置 Provider,要么统一在页面更上层放置单个 Provider。

Copilot uses AI. Check for mistakes.
Comment on lines +216 to +218
<SelectItem value="createdAt">{t('dataList.sortField.createdAt')}</SelectItem>
<SelectItem value="mountCount">{t('dataList.sortField.mountCount')}</SelectItem>
</SelectContent>
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】DataList 这里新增了 dataList.sortField.* / dataList.sortDirection.* 等 i18n key,但目前仅在 zhCN/enUS 增补了翻译,ja/ko locale 中缺少这些 key,会导致对应语言界面显示原始 key 文本。建议补齐 ja/ko 的同名翻译条目,或在代码侧提供合理的 fallback。

Copilot uses AI. Check for mistakes.
@LeiXZ
Copy link
Copy Markdown
Contributor Author

LeiXZ commented Apr 28, 2026

image image

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Comment on lines +95 to 100
const { t } = useTranslation()
const [sort, setSort] = useState('descending')
const [sortField, setSortField] = useState<'createdAt' | 'mountCount'>('mountCount')
const [modelType, setModelType] = useState('所有标签')
const [searchTerm, setSearchTerm] = useState('')
const [ownerFilter, setOwnerFilter] = useState('所有') // 修改默认值为"所有"
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】该组件虽然已引入 i18n(useTranslation),但仍保留了硬编码中文状态值(如“所有标签”“所有”),且下方还有多处硬编码文案(placeholder/描述/按钮/aria 文本等)。这会导致多语言切换不完整;建议把这些展示给用户的字符串统一迁移到 translation.json,并避免用中文字符串作为业务状态值(可改为枚举值 + t() 显示文案)。

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +96 to +97
const [sort, setSort] = useState('descending')
const [sortField, setSortField] = useState<'createdAt' | 'mountCount'>('mountCount')
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【优化建议】DataList 作为通用列表组件(也用于作业模板页等不含 mountCount 的数据),这里把默认排序字段固定成 mountCount 会导致 UI 默认显示“按挂载次数排序”但实际数据不一定支持该字段,容易引起误解。建议:根据 items 是否存在 mountCount 决定默认 sortField,或把可选排序字段作为 props 由调用方传入。

Copilot uses AI. Check for mistakes.
Comment on lines +344 to +347
<div className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium">
<BarChart3Icon className="size-4" />
<span>{item.mountCount}</span>
</div>
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】TooltipTrigger 这里用的是非可聚焦的

(asChild),键盘用户无法 focus 触发 Tooltip,影响可访问性。建议改为 (或至少加 tabIndex=0/role="button" 并处理键盘事件),确保 hover 与 focus 都能触发提示。

Suggested change
<div className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium">
<BarChart3Icon className="size-4" />
<span>{item.mountCount}</span>
</div>
<button
type="button"
className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 border-0 bg-transparent p-0 text-left text-xs font-medium"
>
<BarChart3Icon className="size-4" />
<span>{item.mountCount}</span>
</button>

Copilot uses AI. Check for mistakes.
Comment on lines +1095 to +1100
{
ID: "202604141700",
Migrate: func(tx *gorm.DB) error {
type Dataset struct {
MountCount int `gorm:"column:mount_count;not null;default:0;comment:mount count"`
}
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【优化建议】该迁移的 ID(202604141700)时间戳早于前面已有的 202604161300 / 202604171200,但却追加在列表末尾。虽然 gormigrate 按列表顺序执行不一定出错,但会破坏“ID 随时间递增”的可读性,并可能给后续迁移排查/回滚带来困扰;建议把 ID 调整为当前最新时间点并保持单调递增。

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@AkashiSensei AkashiSensei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好棒,但是再稍微看一下下好了。

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要确认一下 Copilot 提示的内容,确保对这个高复用组件的修改不会影响其它页面。

Comment thread frontend/.gitattributes
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是在 Windows 上开发遇到换行符相关的问题了嘛

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我这边之前设置了一下全局的配置就没有问题了,这次是在pr-commit-check的时候遇到了问题,然后就加了一下

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Comment thread frontend/Makefile
WHITE := \033[37m
RESET := \033[0m

PYTHON ?= $(shell if command -v python3 >/dev/null 2>&1 && python3 -c "import sys" >/dev/null 2>&1; then echo python3; elif command -v python >/dev/null 2>&1; then echo python; fi)
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】【best_practices】这里的 PYTHON 自动探测会在系统仅存在 python(=Python2) 时返回 python,但 hack/format_translation.py 使用了 f-string 等 Python3 语法,会导致 pre-commit-check / format-translation / generate-error-code 在该环境下直接失败。建议把探测条件改为“必须是 Python3”(例如通过 python -c "import sys; exit(0 if sys.version_info[0]==3 else 1)" 校验),或仅回退到 python 时也做版本检查。

Suggested change
PYTHON ?= $(shell if command -v python3 >/dev/null 2>&1 && python3 -c "import sys" >/dev/null 2>&1; then echo python3; elif command -v python >/dev/null 2>&1; then echo python; fi)
PYTHON ?= $(shell if command -v python3 >/dev/null 2>&1 && python3 -c "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)" >/dev/null 2>&1; then echo python3; elif command -v python >/dev/null 2>&1 && python -c "import sys; sys.exit(0 if sys.version_info[0] == 3 else 1)" >/dev/null 2>&1; then echo python; fi)

Copilot uses AI. Check for mistakes.
Comment on lines +1096 to +1102
ID: "202604141700",
Migrate: func(tx *gorm.DB) error {
type Dataset struct {
MountCount int `gorm:"column:mount_count;not null;default:0;comment:mount count"`
}
return tx.Migrator().AddColumn(&Dataset{}, "MountCount")
},
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【核心规范】【operational_implications】migration ID 202604141700 被追加在 202604171200 之后但时间戳更早,容易造成迁移历史混乱(排查问题/回滚/对账时难以按时间理解),也可能影响依赖“ID 递增”习惯的工具或流程。建议将该迁移 ID 调整为当前迁移序列中递增的最新值,并保持按时间递增的命名习惯。

Copilot uses AI. Check for mistakes.
Comment on lines +365 to +371
<span
className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium"
tabIndex={0}
>
<BarChart3Icon className="size-4" />
<span>{item.mountCount}</span>
</span>
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

【优化建议】【accessibility】mountCount 这里用 <span> 做 Tooltip 触发器时,读屏可能只会读到数字而缺少语义;同时你已经新增了 dataList.mountCount 的带参数文案但当前未使用。建议给触发器补充可访问性名称(例如 aria-label/aria-describedby,或改用语义化的 <button>),并考虑用 t('dataList.mountCount', { count: item.mountCount }) 作为可读文本/aria-label,从而复用现有多语言条目。

Suggested change
<span
className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium"
tabIndex={0}
>
<BarChart3Icon className="size-4" />
<span>{item.mountCount}</span>
</span>
<button
type="button"
aria-label={t('dataList.mountCount', { count: item.mountCount })}
className="text-muted-foreground hover:text-foreground inline-flex cursor-help items-center gap-1 text-xs font-medium"
>
<BarChart3Icon className="size-4" aria-hidden="true" />
<span>{item.mountCount}</span>
</button>

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants