[to-reply] feature: support saga multi type config #741
Conversation
| @@ -0,0 +1,127 @@ | |||
| package parser | |||
There was a problem hiding this comment.
要带上apache的License 头，参考别的文件copy一个，或者参考根目录的 goimports.sh 下面所有的文件都检查一下
pkg/saga/statemachine/statelang/parser/statemachine_config_parser.go
Outdated
Show resolved
Hide resolved
pkg/saga/statemachine/statelang/parser/statemachine_config_parser.go
Outdated
Show resolved
Hide resolved
|
配置文件的解析，可以复用当前已有的逻辑 |
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| _, err := parser.Parse(tt.configFilePath) |
pkg/saga/statemachine/statelang/parser/statemachine_config_parser_test.go
Show resolved
Hide resolved
| @@ -115,16 +112,3 @@ func (stateMachineParser JSONStateMachineParser) isTaskState(stateType string) b | |||
| } | |||
| return false | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
StateMachineJsonObject 改为了 statemachine_config_parser 中的 StateMachineObject，给 json 和 yaml 共用，两种格式解析后的结构是一致的，这里的 Parser 实际是和配置文件格式无关的，所以感觉放在 config parser 中更合理一些
|
|
apache License 要在你所有新增的文件上加上，只要没有的，都要加上 |
好的 |
| // just use float64 to convert, json reader will read all number as float64 | ||
| valueAsFloat64, ok := value.(float64) | ||
| if !ok { | ||
| // use float64 conversion when the configuration file is json, and use int conversion when the configuration file is yaml |
There was a problem hiding this comment.
Why is it designed this way
There was a problem hiding this comment.
Because "encoding/json" package will convert all numerical values into float64 types when parsing json, but" yaml.v3" package will correspond to float64 and int types when parsing YAML, and the modification here is mainly to be compatible with the two formats, otherwise some errors will occur.
| return &stateMachineObject, nil | ||
| } | ||
|
|
||
| type StateMachineObject struct { |
There was a problem hiding this comment.
There was a stateMachineJsonObject struct, maybe we can unify them?
There was a problem hiding this comment.
Yes, the StateMachineObject here is the original StateMachineJsonObject. Because the products of the two formats are basically consistent, I moved the definition of the structure to config parser and added a tag to be compatible with the two formats.
luky116
changed the title
| var stateMachineJsonObject StateMachineJsonObject | ||
|
|
||
| err := json.Unmarshal([]byte(content), &stateMachineJsonObject) | ||
| func (stateMachineParser JSONStateMachineParser) Parse(configFilePath string) (statelang.StateMachine, error) { |
There was a problem hiding this comment.
Make interface and the impl has same param
What this PR does:
Supported multi-type configuration parser in saga mode, fixed some related problems, adjusted and improved related unit tests, and supplemented test data in yaml format.
Which issue(s) this PR fixes:
Fixes #734
Special notes for your reviewer:
All relevant unit tests have been completed and passed.
Does this PR introduce a user-facing change?: