Skip to content

feature: dag support mysql store - #21

Open
Wenne wants to merge 73 commits into
ShiningRush:masterfrom
Wenne:develop
Open

Wenne wants to merge 73 commits into
ShiningRush:masterfrom
Wenne:develop

Conversation

@Wenne

@Wenne Wenne commented May 10, 2023

Copy link
Copy Markdown

支持 mysql 存储

Wenne added 2 commits May 9, 2023 11:52
* add gorm define to entity
* add store for mysql
* add keeper for mysql
@Wenne

Wenne commented May 10, 2023 •

Copy link
Copy Markdown
Author

@ShiningRush 大佬有空可以 review 一下

* add store for mysql
* add keeper for mysql
@ShiningRush

Copy link
Copy Markdown
Owner

好的,我周末看下,测试用例看起来有部分失败了,可以先修复下

@Wenne

Wenne commented May 11, 2023

Copy link
Copy Markdown
Author

好的,我周末看下,测试用例看起来有部分失败了,可以先修复下

已修复~

@ShiningRush

Copy link
Copy Markdown
Owner

@Wenne hi,还麻烦补充下集成测试用例,参考之前的 store 和 keeper 的用例集即可,CI配置可以参考 #24

Comment thread go.mod Outdated
Comment thread keeper/mysql/mysql.go Outdated

const LeaderKey = "leader"

// Keeper mongo implement

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

typo

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修改

Comment thread pkg/entity/dag.go Outdated
Comment thread pkg/exporter/collector.go Outdated
)
}

// todo: just add metrics, register used by existed http server

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

这个todo是?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已剔除

Comment thread keeper/mysql/entity.go Outdated
Comment thread pkg/entity/task.go Outdated

type TaskInstanceParams map[string]interface{}

type TaskInstanceDependOn []string

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

以上三个结构的定义均是通用的,不用加上TaskInstance的前缀吧

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

这边之前是为了适配 gorm 序列化必须要 struct 并且实现 json 解析的一些方法才行,但是新办法不需要了,加上 serializer:json 即可。。我这边去掉

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修改

Comment thread keeper/mysql/entity.go Outdated
type Election struct {
ID string `gorm:"primaryKey;type:VARCHAR(256);not null"`
WorkerKey string `gorm:"type:VARCHAR(256);not null"`
UpdatedAt time.Time `gorm:"autoUpdateTime;type:timestamp;index;"`

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

与 L10 相同

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修改

Comment thread pkg/entity/dag.go Outdated
Save func(data *ShareData) error `json:"-" bson:"-"`

mutex sync.Mutex
mutex sync.Mutex `json:"-" bson:"-"`

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

私有化字段不会被序列化,这个tag是无用的

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修改

Comment thread pkg/entity/dag.go Outdated
type ShareData struct {
Dict map[string]string
Save func(data *ShareData) error
Save func(data *ShareData) error `json:"-" bson:"-"`

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

由于ShareData实现了 JsonMarshal 和 BsonMarshal 方法,因此不需要tag来控制序列化

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修改

Comment thread pkg/entity/dag.go Outdated

// Render variables
func (vars DagInstanceVars) Render(p map[string]interface{}) (map[string]interface{}, error) {
func (vars *DagInstanceVars) Render(p map[string]interface{}) (map[string]interface{}, error) {

@ShiningRush ShiningRush May 15, 2023 •

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

这里调整成引用类型的目的是什么
DagInstanceVars 是一个 map, map本身即为引用类型,因此定义一个指向引用的指针没有太大价值,反而导致 L276 必须要显式去做寻址后才能进行 for range

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修改

@Wenne

Wenne commented May 22, 2023

Copy link
Copy Markdown
Author

工作时候事情有点多。。我周末再提交 pr 一个个修复下 comments

@Wenne Wenne closed this May 22, 2023
@Wenne Wenne reopened this May 22, 2023
@ShiningRush

ShiningRush commented May 23, 2023 •

Copy link
Copy Markdown
Owner

工作时候事情有点多。。我周末再提交 pr 一个个修复下 comments

不急,慢慢弄,我最近事情比较多,也都是只有周末才能仔细瞅瞅代码 😄

@super-sunshines

Copy link
Copy Markdown

大佬你们还更新不!

@ShiningRush

Copy link
Copy Markdown
Owner

我这边可以继续review,主要是owner应该没时间投入了

tianliyun.tly and others added 2 commits May 27, 2026 17:13
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.

5 participants