-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Client: Adds Workflow-Reset API #687
base: main
Are you sure you want to change the base?
Conversation
We intentionally do not expose reset as a high-level client call in any SDK as it is considered an advanced/operator call and not one expected to be invoked regularly (well we kinda expose in Go but make you use the proto objects there too). There are several other calls we don't expose high-level client calls for either (e.g. Out of curiosity, can you give more details on your programmatic reset use case? |
@cretz cheers for looking into this! Let me start with this:
While I cannot give you the details due to NDA, I'm just going to cook one example up (so pls imagine this was a serious example instead of a contrived one it actually is). The main idea is that we need a workflow which can be reset to prior point by the end user. The end user doesn't know this is temporal behind the scenes and so on. We provide an overlay DSL and graphs with lineages so the user actually sees that Like imagine a web-form where each page is backed by a temporal activity and subsequent page can only be filled once the current page is submitted because it depends on some data from the prev page. However the user is free to go back to any page and edit and redo things from there (ofc losing all the work from the point of rewind).
I don't see what is "advanced" about it, can you please explain the perils of exposing it? The way I see it is, I can very easily do this via the If there are critical drawbacks to using it, it should a part of documentation, just like I'm sure it would have been a part of the
That would be annoying UX for devs. I hope you reconsider. But ofc my points are based on (1) my lack of understanding of what is so "advanced" about this (2) critical drawback in using this feature (which otherwise has been working perfectly for me for nearly a week now and clients are happy). So if you answer those then I might educate myself.
I didn't even know |
Reset is meant for workflow-stuck situations and to be manually/explicitly performed by an operator on individual workflows. If you have a user need like you have, you should consider having this in your workflow logic. Maybe that's a loop that is re-iterated based on some user signal. It is clearer/safer in code to model the ability for users to restart some activities than to rely on the blunt reset instrument.
This would be much better modeled in workflow logic than using reset to go back. You can use a simple state machine and/or keep track of the steps completed and discard ones if you want to jump back or whatever. Can share/discard state however you might. Think of how you might write this in Python without Temporal from a Python logic approach. |
But isn't that what temporal is doing for me out of the box when I use the Reset feature? If I wasn't using temporal then I would be doing tons more of state management anyway and yes would have no choice but to state-manage this too. But I am using temporal and it does this bit of state-management for me OOB, so why can't I leverage it? I can understand if you didn't intend this to be a use case while designing the feature but I still don't see the:
Which is where my confusion is. You are asserting a few things (and I acknowledge it), I'm just asking you to evidence it. Eg. (1) Why cleaner? (2) Why safer? (3) What's so blunt about reset that relying on it is being frowned upon and I'm being asked to write potentially a lot more boilerplate in application logic? |
What was changed
The client
diddidn't have a reset workflow api exposed, although the implementation modules have the necessary bits. This PR add the reset API.Why?
This is just like any other missing api (eg. If
terminate()
was missing). Without this I need to create boilerplate to reset a workflow in my python codebase. However since that is external to this library it doesn't get the exact same treatment. For eg. the client-impl in this library is wrapped in a series of interceptors if interceptors were provided. But an external code doesn't benefit from any of this, unless we write even more boilerplate.Checklist
Closes
How was this tested:
This is the code similar to what I implemented in my application and that works.