Plugin support #2135
Plugin support #2135
Conversation
yuandrew
left a comment
yuandrew
left a comment
There was a problem hiding this comment.
Solid! Mainly questions for my own learning, and a few leftover doc TODOs
|
|
||
| // Checks if `line` is a valid definition, and that definition is for `private` | ||
| func isValidDefinitionWithMatch(line, private string, inGroup string, insideStruct bool) bool { | ||
| // Vars with underscores are often used to assert interface validation and |
| // PluginConfigureWorkerRegistryOptions are the set of callbacks that can | ||
| // be adjusted by plugins when configuring workers. If adjusting a callback that | ||
| // is already set, implementers may want to take care to invoke the existing | ||
| // callback inside their own. |
There was a problem hiding this comment.
I'm not sure I understand the last sentence or why it needs a special callout, this is only if they want the newly adjusted callback to continue to do the old callback's logic, on top of what's new right? If they want to adjust their plugin callback to do something completely different, then they wouldn't want to do this?
There was a problem hiding this comment.
Right. But I figured it was important to call out that if they set this, they overwrite what was set by other plugins, because at first glance it could look like each plugin can set this uniquely unrelated to each other.
|
|
||
| // RunContextBefore is invoked on worker start or before each workflow | ||
| // replay of a replayer. Implementers can use this to register items or | ||
| // simply start something needed. |
There was a problem hiding this comment.
register items
refers to registering things onto the Context, right?
simply start something needed
maybe more of a nit: this phrase feels strange to me, maybe because it feels a little vague and I'm not able to imagine what an example is
There was a problem hiding this comment.
refers to registering things onto the Context, right?
No, the options
| type WorkerPluginConfigureWorkflowReplayerOptions struct { | ||
| // WorkflowReplayerInstanceKey is the unique, immutable instance key for | ||
| // this workflow replayer. | ||
| WorkflowReplayerInstanceKey string |
There was a problem hiding this comment.
This is new, I don't think any of the core langs have this. Is there any utility of this I'm not seeing besides giving each replay an ID and allowing tests to verify Plugins are being applied to replay in the correct sequence?
Correct me if I'm wrong, but each time a worker replays a workflow, it'll generate a new WorkflowReplayerInstanceKey. So A single worker, with its own WorkerInstanceKey, can generate many new WorkflowReplayerInstanceKeys
There was a problem hiding this comment.
The core langs don't have this because they will be getting access to the actual worker/replay instance in their "run worker" or "replay workflow" calls (which has the options and such). This is needed if a user needs to correlate configuration with the instance.
Not specific to this but for all XInstanceKeys in this PR, there needs to be a way for workers to correlate start with stop (and configure, though that's less important) and workflow replayers to correlate successive replay runs together. The worker side is the most important, it's just that I went ahead and added it to replayer for clarity there too.
For instance, say your plugin on worker start registered an activity with a newly opened database connection. When should it close that database connection?
I do expect this to be the same value as the actual instance key we're using for worker heartbeating when we do that at some point, but since this is all experimental (eek, I need to go mark them all as such), we can adjust as needed in the future.
| // to invoke the existing callback inside their own. | ||
| // | ||
| // Exposed as: [go.temporal.io/sdk/worker.PluginConfigureWorkflowReplayerRegistryOptions] | ||
| type WorkerPluginConfigureWorkflowReplayerRegistryOptions struct { |
There was a problem hiding this comment.
Is this the longest var name in the SDK? 😂
2 participants
What was changed
Added
client.Plugin,worker.Plugin, andtemporal.SimplePluginand everything required to make those work.Notes:
Checklist