Add links and callbacks support for standalone activity#759
Add links and callbacks support for standalone activity#759
Conversation
Added links/callbacks to describe and start responses Update openapi
Co-authored-by: Quinn Klassen <klassenq@gmail.com>
Co-authored-by: Spencer Judge <spencer@temporal.io>
fretz12
changed the title
dandavison
left a comment
dandavison
left a comment
There was a problem hiding this comment.
Looks great but I think that we need to make CallbackInfo handle Activity before merging.
| bytes long_poll_token = 5; | ||
|
|
||
| // Callbacks attached to this activity execution and their current state. | ||
| repeated temporal.api.workflow.v1.CallbackInfo callbacks = 6; |
There was a problem hiding this comment.
This CallbackInfo is currently workflow-specific:
// CallbackInfo contains the state of an attached workflow callback.
message CallbackInfo {
// Trigger for when the workflow is closed.
message WorkflowClosed {}
message Trigger {
oneof variant {
WorkflowClosed workflow_closed = 1;
}
}We either need to make it more general, or add an activity-specific CallbackInfo. I'm thinking we'd want the former -- add an ActivityClosed or ExecutionClosed trigger. We're going to want to straighten this out, and it would be a breaking API change to do it later.
| repeated temporal.api.common.v1.Callback completion_callbacks = 19; | ||
| // Links to be associated with the activity. Callbacks may also have associated links; | ||
| // links already included with a callback should not be duplicated here. | ||
| repeated temporal.api.common.v1.Link links = 20; |
There was a problem hiding this comment.
The SAA PR only uses links on the callback. Is it necessary/appropriate to add these top-level links in addition to the callback links? If so, could the Nexus team give a refresher on what the respective roles are of the two links (I had it in my head that top-level were deprecated).
| // Set if activity cancelation was requested. | ||
| string canceled_reason = 32; | ||
|
|
||
| // Links associated with the activity. |
There was a problem hiding this comment.
A reader is going to think initially these are links that in some way refer to this activity. However, they're not -- they're links pointing to other entities associated with the activity; typically the entity that started the activity. Let's clarify this for the reader, here and in the workflow counterpart HistoryEvent.links. As an author of a Nexus SDK I found this confusing at first, so there's not much doubt that other users will benefit from some help here.
| // Links associated with the activity. | |
| // Links to entities associated with this activity, for example | |
| // to the entity that started this activity. |
5 participants
What changed?
Why?
To support links and callbacks for standalone activity executions (SAA), bringing feature parity with workflow-based activity scheduling.