Conversation
- add config file - naive approach
- fix tag field - check type
- Added feature start a "process" with a file - Added feature create all resources and start all processess into a folder -Added sample config files
There was a problem hiding this comment.
Looking pretty good - just two major comments
- Could we simplify the logic in the cmd files e.g.
Rework the functionality into to 4 separate bits for each command -
- load all the configs - -f there is one, -d there is one or more, no -f or -d construct from the command line or load through stdin. Your struct cliConfStruct looks like it could be useful here
- order them - depending on the command I think "process" either go first or last or it doesn't matter
- validate the configs
- send them over the wire
I think you have got all of the bits - I would just like to see them expressed in the commands
-
- A bit of code duplication round loading yaml.
cmd/device-hub-cli/helpers.go
Outdated
| return f, dm.NewDecoder(f), nil | ||
| } | ||
|
|
||
| func yamlDecoder(filePath string, target interface{}) error { |
There was a problem hiding this comment.
Duplicate function with helpers.go - line 132.
cmd/device-hub-cli/cmd_create.go
Outdated
| Tags: map[string]string{}, | ||
| } | ||
| tags := []string{} | ||
| return startCall(args, request, tags, cli, in, out) |
There was a problem hiding this comment.
I'm really not keen on this case statement - lines 52 to 58 :
- the other two cases are for validation whilst this specializes for the end of the function e.g. does almost the same bits of code just different.
cmd/device-hub-cli/helpers.go
Outdated
| sort.Sort(cliConfSlice(dataSlice)) | ||
| case "delete": | ||
| sort.Sort(sort.Reverse(cliConfSlice(dataSlice))) | ||
| } |
There was a problem hiding this comment.
I'm not 100 sure that this switch on the caller is required. I understand what you are trying to do - its just leading to a bit of spaghetti code. Maybe the ordering could be expressed clearer before the roundtrip function?
cmd/device-hub-cli/helpers.go
Outdated
| } | ||
| } | ||
|
|
||
| type roundTripFunc func(cli proto.HubClient, in rawConf, out iocodec.Encoder) error |
There was a problem hiding this comment.
If I understand correctly a rawConf is just a yaml doecoder - is it any different to the iocodec.Decoder for "yaml"?
cmd/device-hub-cli/helpers.go
Outdated
| return nil | ||
| } | ||
|
|
||
| type cliConfSlice struct { |
There was a problem hiding this comment.
I am wondering if you could restructure the code to on each command return a cliConfSlice, order as per the intention of the command, validate, send over the wire? Please see the overall review comment.
2 participants
Preliminar PR addresses #79 and adds:
./device-hub-cli create -d <folder>./device-hub-cli delete -d <folder>To start a process using a config file:
./device-hub-cli start -f=../../../examples/config/http_listener_process.yamlTODO: