temporalio / sdk-dotnet

Temporal .NET SDK
MIT License
373 stars 30 forks source link

Handle multiple completion commands #305

Open dandavison opened 1 month ago

dandavison commented 1 month ago

The change made in https://github.com/temporalio/sdk-dotnet/pull/270 is incorrect when there are multiple completion commands: in that case, we may still leave non-query commands after a completion command, and currently core removes any such commands.

We are going to fix this in core (https://github.com/temporalio/sdk-core/issues/778). This ticket is blocked on the core work but we anticipate the fix here to be straightforward: send all commands to core i.e. with (potentially multiple) completion commands interleaved with non-completions.

cretz commented 1 month ago

:+1: Note as part of this work we still have to leave existing logic, but only if this flag is present. I am unsure how safe we feel about removing the old .NET logic of truncating post-completion commands (since Core was doing it too), up for discussion. It would be nice if the .NET command reordering we did here was only if this flag was set and we could leave everything else (including truncation) up to Core.