temporalio / sdk-python

Temporal Python SDK
MIT License
474 stars 77 forks source link

[Feature Request] Decode json/plain as json/protobuf if type_hint is a subclass of protobuf.Message #452

Closed ndtretyak closed 9 months ago

ndtretyak commented 11 months ago

Is your feature request related to a problem? Please describe.

All my workflows accepts protobufs as arguments and it works fine if I start them using SDK's client since it adds json/protobuf as payload's encoding. But sometimes it is useful to start workflows using tctl that usesjson/plain encoding.

Now, JSONPlainPayloadConverter, which is used for the json/plain encoding, does not handle proto.Messages in type_hints and just fails.

Describe the solution you'd like

I'd like JSONPlainPayloadConverter to handle json-encoded protobufs.

cretz commented 11 months ago

sometimes it is useful to start workflows using tctl that usesjson/plain encoding.

This is a problem with tctl not the SDK. We need the ability to support saying the metadata encoding of inputs (https://github.com/temporalio/tctl/issues/185). I believe once the newer incarnation of the Temporal CLI is developed, it will support setting the encoding.

I'd like JSONPlainPayloadConverter to handle json-encoded protobufs.

You can do this with a custom converter, but I hesitate to do this by default. We only want the JSON proto converter to have special rules about proto.

cretz commented 9 months ago

Closing (not adding proto support to general json/plain converter), but be advised that the new Temporal CLI before it reaches 1.0 GA will support supplying protos.