temporalio / proposals

Temporal proposals
https://temporal.io
MIT License
69 stars 18 forks source link

Workflow Task Completion Metadata #73

Closed Sushisource closed 1 year ago

Sushisource commented 1 year ago

Rendered

cretz commented 1 year ago

I need to understand the specific problem better to continue with a review. The solution here is very generic, but the only concrete use case I see is getting the version of the SDK that produced a specific WFT, and I'm wondering why we even need that.

@MichaelSnowden - I think the best way to describe it is "markers that don't take an event". Markers too were very generic and we use those for versioning in SDK-specific ways. The specific initial concrete use case is simple - we need to record SDK version on workflow task completion so that successive workflow tasks can make decisions about what may nor may not be compatible since the start of a workflow. But we may have other uses cases - just like markers were built for future use cases. A general solution applicable to future uses makes sense.

Sushisource commented 1 year ago

So, we talked more within the SDK team and we decided that the flexibility here comes at a bit too high of a cost in terms of size. We don't actually have any concrete use cases at the moment that require writing completely arbitrary data. Everything we want to write in here is well defined for at least one consumer (even if it's only the SDK consuming its own notes).

Hence, everything we know we want to add here is already a proto. The only question is how it will get encoded. (Hence the desire for Any in my original version). But, both Any and Payload are byte-wasteful.

The main motivation for adding this was speculation about needing to add generic stuff (but we don't know what that is) and decoupling SDK-only stuff from the long approval process / lifecycle in this repo. The first we can deal with if/when it comes up. The second we can deal with by simply introducing an sdk package in this repo with different codeowners. We'll do that.