Forwarded JSOn should have lowercase keys#11
Conversation
|
Replaces #4 by adding some tests |
Golang uses capitalized variable names for JSON that is sent into the yggdrasil service. When this data is forwarded, it needs to be converted back to the proper JSON format to retain the same structure.
|
Bug tracked in SAT-10933. |
|
@ShimShtein Could you give this a review? |
adamruzicka
left a comment
adamruzicka
left a comment
There was a problem hiding this comment.
LGTM apart from the error handling which in general feels lacking
| dataJson, error := json.Marshal(data) | ||
| if error != nil { | ||
| log.Fatal(error) | ||
| } |
There was a problem hiding this comment.
I know this is somewhat unlikely to happen and that this is just the error handling that was here before moved into a function, but it doesn't feel right. If the data cannot be encoded as json, we'd log an error message and send a post with an empty body to the server? Shouldn't it report the error back to the cloud and bail out?
There was a problem hiding this comment.
Fatal method logs and exits in the same command: https://pkg.go.dev/log#Fatal
There was a problem hiding this comment.
That's a bit surprising. Anyway, the worker dies and most likely gets restarted, but is the cloud notified somehow?
There was a problem hiding this comment.
It depends on yggdrasil's error handling for the Send method. The method call would be aborted, so it will result in an error on the dispatcher side.
There was a problem hiding this comment.
Soooo, what is my take away here?
There was a problem hiding this comment.
Let's leave it as is and hope yggdrasil can somehow deal with it I guess
|
Merged, thanks @ehelms ! |
4 participants
Golang uses capitalized variable names for JSON that is sent into
the yggdrasil service. When this data is forwarded, it needs to be
converted back to the proper JSON format to retain the same structure.