Support multiple --input-meta flags for the same key#836
Support multiple --input-meta flags for the same key#836bergundy merged 5 commits intotemporalio:next-serverfrom
Conversation
bergundy
force-pushed
the
multi-signal-input
branch
2 times, most recently
from
3ddd256 to
d3151e3
Compare
| ) | ||
|
|
||
| func CreatePayloads(data [][]byte, metadata map[string][]byte, isBase64 bool) (*common.Payloads, error) { | ||
| func CreatePayloads(data [][]byte, metadata map[string][][]byte, isBase64 bool) (*common.Payloads, error) { |
There was a problem hiding this comment.
It's a little confusing following this code. May need some godoc on this method explaining this map[string][][]byte data structure. As it looks today, I'd call this expecting 1:1 with data, but that's not the case. Rather it seems that the first array value is applied unless there is one for the data index. So if I have 3 pieces of data and my metadata has 2 values for "encoding", say "foo" and "bar", the first data item and the third data item get "foo" and only the middle one gets "bar".
Not saying this is wrong necessarily, just confusing and worth documenting somewhere clearly so it is clear that the logic below is by intention and what callers should expect. Maybe just toss a "If metadata has an entry at a data index, it is used, otherwise it uses the metadata entry at index 0" above the function would be enough.
There was a problem hiding this comment.
It looks like a comment was made as a result of this comment (it's a bit hard to know without an update to this thread).
temporalcli/payload.go
Outdated
| // If it's JSON, validate it | ||
| if strings.HasPrefix(string(metadata["encoding"]), "json/") && !json.Valid(in) { | ||
| return nil, fmt.Errorf("input #%v is not valid JSON", i+1) | ||
| var metadataForIndex = map[string][]byte{} |
There was a problem hiding this comment.
| var metadataForIndex = map[string][]byte{} | |
| metadataForIndex := make(map[string][]byte, len(metadata)) |
Pedantic, can ignore, but a nice thing to do
| if vals, ok := metadata[metaPieces[0]]; ok { | ||
| if len(vals) == len(inData) { | ||
| return nil, fmt.Errorf("received more --input-meta flags for key %q than number of inputs", metaPieces[0]) | ||
| } | ||
| metadata[metaPieces[0]] = append(vals, []byte(metaPieces[1])) | ||
| } else { | ||
| metadata[metaPieces[0]] = [][]byte{[]byte(metaPieces[1])} | ||
| } |
There was a problem hiding this comment.
| if vals, ok := metadata[metaPieces[0]]; ok { | |
| if len(vals) == len(inData) { | |
| return nil, fmt.Errorf("received more --input-meta flags for key %q than number of inputs", metaPieces[0]) | |
| } | |
| metadata[metaPieces[0]] = append(vals, []byte(metaPieces[1])) | |
| } else { | |
| metadata[metaPieces[0]] = [][]byte{[]byte(metaPieces[1])} | |
| } | |
| metadata[metaPieces[0]] = append(metadata[metaPieces[0]], []byte(metaPieces[1])) | |
| if len(metadata[metaPieces[0]]) > len(inData) { | |
| return nil, fmt.Errorf("received more --input-meta flags for key %q than number of inputs", metaPieces[0]) | |
| } |
Pedantic, but for a map of slices, it's a bit more idiomatic Go to append to the nil result of the absent key rather than have conditionals (can use intermediate vars if you are concerned with the accesses)
bergundy
force-pushed
the
multi-signal-input
branch
from
8bd599e to
a5ec0c1
Compare
| ) | ||
|
|
||
| func CreatePayloads(data [][]byte, metadata map[string][]byte, isBase64 bool) (*common.Payloads, error) { | ||
| func CreatePayloads(data [][]byte, metadata map[string][][]byte, isBase64 bool) (*common.Payloads, error) { |
There was a problem hiding this comment.
It looks like a comment was made as a result of this comment (it's a bit hard to know without an update to this thread).
Co-authored-by: Spencer Judge <[email protected]>
Closes #833 --------- Co-authored-by: Spencer Judge <[email protected]>
Closes #833 --------- Co-authored-by: Spencer Judge <[email protected]>
Closes #833 --------- Co-authored-by: Spencer Judge <[email protected]>
3 participants
Closes #833