-
Notifications
You must be signed in to change notification settings - Fork 22
Nexus caller-only support #381
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?
Conversation
b2a414c to
a765f1b
Compare
| gem 'rb_sys', '~> 0.9' | ||
| gem 'rdoc' | ||
| gem 'rubocop' | ||
| # TODO: Unpin when https://github.com/rubocop/rubocop/issues/14837 is fixed |
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.
Impressive turnaround
| # TODO: Unpin when https://github.com/rubocop/rubocop/issues/14837 is fixed | |
| # TODO: Unpin when https://github.com/rubocop/rubocop/pull/14838 is released |
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.
It broke everyone, they had to, heh
| # For async operations, operation_token should be present | ||
| Temporalio::Workflow.logger.info("Operation started with token: #{handle.operation_token}") |
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.
Should there be an assertion that the token is present instead of just logging it? I think it is covered by test_nexus_operation_handle_async_has_token and no log or assertion is needed.
| # For async operations, operation_token should be present | |
| Temporalio::Workflow.logger.info("Operation started with token: #{handle.operation_token}") |
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.
Yeah, these logs/comments aren't necessary (I don't think we need specific assertions here, but may add them). Will fix.
| # For async operations, operation_token should be present | ||
| Temporalio::Workflow.logger.info("Operation started with token: #{handle.operation_token}") |
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.
Should there be an assertion that the token is present instead of just logging it? I think it is covered by test_nexus_operation_handle_async_has_token and no log or assertion is needed.
| # For async operations, operation_token should be present | |
| Temporalio::Workflow.logger.info("Operation started with token: #{handle.operation_token}") |
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.
Yeah, these logs/comments aren't necessary (I don't think we need specific assertions here, but may add them). Will fix.
What was changed
Added workflow caller support for Nexus. This does not include implementing Nexus handlers. Specifically:
Temporalio::Workflow.create_nexus_clientthat returns Nexus clientTemporalio::Workflow::NexusClientwithstart_operationandexecute_operationthat work with operation handlesTemporalio::Workflow::NexusOperationHandleto support fetching resultTemporalio::Error::NexusOperationErrorandTemproalio::Error::NexusHandlerErrorand conversion code in each directioncreate_nexus_endpointanddelete_nexus_endpointtoTemporalio::Testing::WorkflowEnvironment