imbajin commented on code in PR #336:
URL:
https://github.com/apache/incubator-hugegraph-computer/pull/336#discussion_r2444661235
##########
vermeer/apps/master/master_main.go:
##########
@@ -56,6 +56,7 @@ func Main() {
services.SetUI(sen)
logrus.Info("token-auth was activated")
default:
Review Comment:
**代码风格**: 这里有一个空的 default 分支并且后面有多余的空行。应该删除或添加注释说明为什么需要空的 default 分支。
**建议**:
```go
default:
// No authentication required for other routes
```
或者如果不需要这个 default 分支,可以直接删除。
##########
vermeer/apps/master/bl/task_bl.go:
##########
@@ -62,6 +65,56 @@ func (tb *TaskBl) CreateTaskInfo(
return nil, err
}
+ // for scheduler
+ taskInfo.Priority = 0
+ taskInfo.Preorders = make([]int32, 0)
+ taskInfo.Exclusive = true // default to true for now, can be set false
by params
+ if params != nil {
+ if priority, ok := params["priority"]; ok {
+ if p, err := strconv.ParseInt(priority, 10, 32); err ==
nil {
+ if p < 0 {
Review Comment:
**类型转换安全性**: 虽然已经检查了 p < 0,但还应该检查转换后的值是否会溢出。
**建议**:
```go
if p < 0 || p > math.MaxInt32 {
return nil, fmt.Errorf("priority %d out of valid range [0, %d]", p,
math.MaxInt32)
}
taskInfo.Priority = int32(p)
```
这样可以防止在 32 位系统上出现意外的溢出问题。
##########
vermeer/apps/master/bl/task_bl.go:
##########
@@ -62,6 +65,56 @@ func (tb *TaskBl) CreateTaskInfo(
return nil, err
}
+ // for scheduler
+ taskInfo.Priority = 0
+ taskInfo.Preorders = make([]int32, 0)
+ taskInfo.Exclusive = true // default to true for now, can be set false
by params
+ if params != nil {
+ if priority, ok := params["priority"]; ok {
+ if p, err := strconv.ParseInt(priority, 10, 32); err ==
nil {
+ if p < 0 {
+ return nil, fmt.Errorf("priority should
be non-negative")
+ }
+ if p > math.MaxInt32 {
+ return nil, fmt.Errorf("priority
exceeds maximum value: %d", math.MaxInt32)
+ }
+ taskInfo.Priority = int32(p)
+ } else {
+ logrus.Warnf("priority convert to int32
error:%v", err)
+ return nil, err
+ }
+ }
+ if preorders, ok := params["preorders"]; ok {
+ preorderList := strings.Split(preorders, ",")
+ for _, preorder := range preorderList {
+ if pid, err := strconv.ParseInt(preorder, 10,
32); err == nil {
+ if taskMgr.GetTaskByID(int32(pid)) ==
nil {
Review Comment:
**错误消息国际化**: 错误消息使用了中文和英文混合。建议统一使用英文,或者实现完整的国际化支持。
**示例**:
```go
// 当前
return nil, fmt.Errorf("preorder task id %d not exists", depTaskID)
// 建议改为统一英文
return nil, fmt.Errorf("preorder task id %d does not exist", depTaskID)
```
或者实现国际化框架:
```go
return nil, i18n.Errorf("error.task.dependency_not_found", depTaskID)
```
##########
vermeer/apps/master/bl/task_creator.go:
##########
@@ -99,6 +99,28 @@ func (bc *baseCreator) NewTaskInfo(graphName string, params
map[string]string, t
return task, nil
}
+func (bc *baseCreator) CopyTaskInfo(src *structure.TaskInfo)
(*structure.TaskInfo, error) {
Review Comment:
**测试覆盖率**: 这个函数复制了任务信息,但清除了 cron 表达式。这个行为需要单元测试来确保:
1. 所有字段都被正确复制
2. cron 表达式确实被清除
3. 新任务 ID 是唯一的
**建议添加测试**:
```go
func TestCopyTaskInfo(t *testing.T) {
original := &structure.TaskInfo{
ID: 1,
CronExpr: "0 0 * * *",
// ... 其他字段
}
copied := CopyTaskInfo(original)
assert.NotEqual(t, original.ID, copied.ID)
assert.Empty(t, copied.CronExpr)
assert.Equal(t, original.Name, copied.Name)
// ... 验证其他字段
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
