chasm(callbacks): create placeholders for chasm callbacks archetype#8473
chasm(callbacks): create placeholders for chasm callbacks archetype#8473
Conversation
| // The time when the next attempt is scheduled. | ||
| google.protobuf.Timestamp next_attempt_schedule_time = 8; |
There was a problem hiding this comment.
My experience with the CHASM port of Scheduler is that a lot of these next_*_time fields go away, since you no longer need to be able to regenerate tasks based on component state. Feel free to leave it for now till implementation though.
| map<string, string> header = 2; | ||
| } | ||
|
|
||
| message CHASM { |
There was a problem hiding this comment.
(minor) We ended up going with Chasm as the casing convention 🤷♀️ . Happy to revisit that but we may want to do it codebase-wide.
| message InvocationTask { | ||
| // The base URL for nexus callbacks. | ||
| // Will have other meanings as more callback use cases are added. | ||
| string destination = 1; |
There was a problem hiding this comment.
TaskAttributes already has a Destination field, referring to the destination outbound queue, which may overload the term a little; maybe simply url or nexus_url?
| } | ||
|
|
||
| message BackoffTask { | ||
| google.protobuf.Timestamp deadline = 1; |
There was a problem hiding this comment.
Similarly, if you want to simply specify the task's schedule time, you can use TaskAttributes.ScheduledTime (example for Scheduler's backfiller bumping its own task). That said, if this deadline is configurable by dynamic config, you'll probably want to store it on the task (as you have) so that the deadline sticks.
There was a problem hiding this comment.
I will leave this for now and included a note that it might get removed in the future
| } | ||
|
|
||
| message BackoffTask { | ||
| google.protobuf.Timestamp deadline = 1; |
There was a problem hiding this comment.
The deadline is part of the task attributes in chasm.
| map<string, string> header = 2; | ||
| } | ||
|
|
||
| message CHASM { |
There was a problem hiding this comment.
nit: call this file callback.go or even just component.go
| } | ||
|
|
||
| func (c *Callback) LifecycleState(_ chasm.Context) chasm.LifecycleState { | ||
| return chasm.LifecycleStateRunning |
There was a problem hiding this comment.
Add a TODO here or just implement this. There are specific states in the enum that are obviously not running states.
| } | ||
|
|
||
| func (c *Callback) State() *callbackspb.CallbackState { | ||
| return c.CallbackState |
There was a problem hiding this comment.
This needs to be a comparable, we typically use an enum for this.
There was a problem hiding this comment.
The library (directory and package) should be called callback singular not callbacks plural.
| ) | ||
|
|
||
| var RequestTimeout = dynamicconfig.NewDestinationDurationSetting( | ||
| "chasm.callbacks.request.timeout", |
There was a problem hiding this comment.
Apply here and everywhere else please.
| "chasm.callbacks.request.timeout", | |
| "chasm.callback.request.timeout", |
| message Chasm { | ||
| // namespace id of the target state machine. | ||
| string namespace_id = 1; | ||
| // ID of the workflow that the target state machine is attached to. | ||
| string workflow_id = 2; | ||
| // Run id of said workflow. | ||
| string run_id = 3; | ||
| // A reference to the component. | ||
| repeated string component_path = 4; | ||
| // Method to invoke on target component | ||
| string method = 5; | ||
| } |
| // Trigger for when the workflow is closed. | ||
| message WorkflowClosed {} | ||
|
|
||
| message Trigger { |
There was a problem hiding this comment.
I think we can probably get rid of this for now, we can add it back when we have more variants if needed.
| // The time when the callback was registered. | ||
| google.protobuf.Timestamp registration_time = 3; | ||
|
|
||
| temporal.server.api.enums.v1.CallbackState state = 4; |
There was a problem hiding this comment.
Inline the enum in this file please. We may want to call this Status instead of State to disambiguate.
3 participants
What changed?
Create the chasm placeholder structs for
Callbackport from HSM -> CHASMWhy?
Port HSM -> CHASM
How did you test it?
Potential risks
None, this code is not used yet