imbajin commented on code in PR #336:
URL:
https://github.com/apache/incubator-hugegraph-computer/pull/336#discussion_r2444646140
##########
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 分支。
**建议**:
```go
default:
// No authentication required for other modes
```
或者如果不需要 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:
**类型转换安全性**: 虽然已经检查了 ,但还应该检查转换后的值是否会溢出。
**建议**:
```go
if p < 0 || p > math.MaxInt32 {
return nil, fmt.Errorf("priority must be between 0 and %d, got %d",
math.MaxInt32, p)
}
```
另外,考虑直接使用 的第三个参数来限制位数,这样就不需要额外检查了。
##########
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 with ID %d does not exist", pid)
// 建议保持一致的风格
```
这对于开源项目尤其重要,因为国际贡献者可能不懂中文。
##########
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) {
// Test case 1: nil input
// Test case 2: all fields copied correctly
// Test case 3: cron expression cleared
// Test case 4: new task ID generated
}
```
--
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]
