-
Notifications
You must be signed in to change notification settings - Fork 94
feature: dag support mysql store #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
* add gorm define to entity
* add store for mysql * add keeper for mysql
@ShiningRush 大佬有空可以 review 一下 |
* add store for mysql * add keeper for mysql
好的,我周末看下,测试用例看起来有部分失败了,可以先修复下 |
已修复~ |
keeper/mysql/mysql.go
Outdated
|
||
const LeaderKey = "leader" | ||
|
||
// Keeper mongo implement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
pkg/exporter/collector.go
Outdated
) | ||
} | ||
|
||
// todo: just add metrics, register used by existed http server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个todo是?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已剔除
pkg/entity/task.go
Outdated
|
||
type TaskInstanceParams map[string]interface{} | ||
|
||
type TaskInstanceDependOn []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
以上三个结构的定义均是通用的,不用加上TaskInstance的前缀吧
There was a problem hiding this comment.
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 即可。。我这边去掉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
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;"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
与 L10 相同
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
pkg/entity/dag.go
Outdated
Save func(data *ShareData) error `json:"-" bson:"-"` | ||
|
||
mutex sync.Mutex | ||
mutex sync.Mutex `json:"-" bson:"-"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
私有化字段不会被序列化,这个tag是无用的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
pkg/entity/dag.go
Outdated
type ShareData struct { | ||
Dict map[string]string | ||
Save func(data *ShareData) error | ||
Save func(data *ShareData) error `json:"-" bson:"-"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
由于ShareData实现了 JsonMarshal
和 BsonMarshal
方法,因此不需要tag来控制序列化
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
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) { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已修改
工作时候事情有点多。。我周末再提交 pr 一个个修复下 comments |
不急,慢慢弄,我最近事情比较多,也都是只有周末才能仔细瞅瞅代码 😄 |
Add dia filter in listdaginstances
Tidb cloud
大佬你们还更新不! |
我这边可以继续review,主要是owner应该没时间投入了 |
支持 mysql 存储