-
Notifications
You must be signed in to change notification settings - Fork 20
Plugin support #370
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
base: main
Are you sure you want to change the base?
Plugin support #370
Conversation
chris-olszewski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything makes sense to me. Appreciate the in depth comments.
| # @param lazy_connect [Boolean] If true, there is no connection until the first call is attempted or a worker | ||
| # is created with it. Clients from lazy connections cannot be used for workers if they have not performed a | ||
| # connection. | ||
| # @param around_connect [Proc, nil] If present, this proc accepts two values: options and a block. The block is a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # @param around_connect [Proc, nil] If present, this proc accepts two values: options and a block. The block is a | |
| # @param around_connect [Proc, nil] If present, this proc accepts two values: options and a block. The block |
| ) | ||
|
|
||
| # Create a client and worker with the plugin, run workflow, confirm success | ||
| client = Temporalio::Client.connect(env.client.connection.target_host, env.client.namespace, plugins: [plugin]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too concerned since the code is pretty straightforward, but adding some test coverage for composing plugins would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may do this, though really, plugins technically shouldn't need/want to know about each other, they should just each work. So effectively it's a test to confirm two plugins work instead of just one, so I may add a second here.
What was changed
Added
Temporalio::Client::Plugin,Temporalio::Worker::Plugin, andTemporalio::SimplePluginand everything required to make those work.Checklist