search

temporalio / features

Behavior and history compatibility testing for Temporal SDKs
14 stars 17 forks source link

Support valid proto JSON in SDK replayers #164

Open bergundy opened 2 years ago

bergundy commented 2 years ago

Our generated Go API uses this patch which causes tooling like the web UI and CLI to generate invalid JSON protos when exporting histories, converting CONSTANT_CASE enum values to CamelCase.

SDK replayers should support the hacked JSON output as well as valid proto JSON output to allow downloading histories programmatically in addition to the CLI and UI. Eventually we could get rid of the hacked JSON output generation and remove the custom conversion code in all SDKs which reverts the hack.

cretz commented 2 years ago

To clarify, most SDKs before using proto3 JSON just need to not add the prefix to the enum if it's already there. It should be a single conditional for most.

Now, in Go this is a different deal. We violated proto3 in the generated code. I am not sure there's an easy path forward to proto3-valid JSON there.

Spikhalskiy commented 2 years ago

I don't think this is the right direction. We already have a format of Temporal History Json that is supported across all the tooling. This format is better than proto3 json for a person reading the histories, because it actually makes enum values much easier to read and follow. Creating several supported formats doesn't solve pretty much any problem, but creates additional maintenance and support burden and unneeded fragmentation.

I think for us as a product it's better in a long run to have our own stable Temporal Histories Format that takes a dependency on a general-purpose format imposed by Google, specific to protobuf version 3, and is not tolerant to any potentially incompatible changes to our proto structures. In comparison to these cons, the maintenance and development burden of our own Temporal History Json format is relatively small.