Execute nexus operation from a workflow #1473
Conversation
internal/interceptor_base.go
Outdated
There was a problem hiding this comment.
May not be needed now, but may want to make an issue to ensure tracing interceptor is updated to support this (and check context propagation).
There was a problem hiding this comment.
context propagation may come later too, for now I'm more concerned with nexus headers (map[string]string).
internal/internal_task_handlers.go
Outdated
| eventAttributes := e.GetNexusOperationScheduledEventAttributes() | ||
| commandAttributes := d.GetScheduleNexusOperationCommandAttributes() | ||
|
|
||
| if eventAttributes.GetService() != commandAttributes.GetService() || eventAttributes.GetOperation() != commandAttributes.GetOperation() { |
There was a problem hiding this comment.
To confirm, we allow endpoint to be changed without failing determinism but not service or operation? One might expect service and endpoint to be the same kind of thing, or at least it may be confusing to explain to users that half of the Nexus client creation can be changed and the other half cannot. I think it should be both endpoint and service or neither. An argument for both is that endpoints can be changed server side.
There was a problem hiding this comment.
Endpoints are env specific, so I'd rather allow this to be changed as long as the "type" is preserved. It's the same as us not checking task queue for activity.
There was a problem hiding this comment.
Unresolving to continue conversation. I think you should either allow the two things that create a client to be changed or disallow the two things that create a client from being changed. Having one of those two changeable is confusing. If you're looking for a task queue analogy, I think that's endpoint + service and if we had an "activity client", creating it would probably have the task queue like the nexus client has endpoint + service.
internal/internal_task_handlers.go
Outdated
| eventAttributes := e.GetNexusOperationScheduledEventAttributes() | ||
| commandAttributes := d.GetScheduleNexusOperationCommandAttributes() | ||
|
|
||
| if eventAttributes.GetService() != commandAttributes.GetService() || eventAttributes.GetOperation() != commandAttributes.GetOperation() { |
There was a problem hiding this comment.
Endpoints are env specific, so I'd rather allow this to be changed as long as the "type" is preserved. It's the same as us not checking task queue for activity.
internal/interceptor_base.go
Outdated
There was a problem hiding this comment.
context propagation may come later too, for now I'm more concerned with nexus headers (map[string]string).
bergundy
force-pushed
the
bergundy/nexus-operation-from-workflow
branch
from
cae90d5
to
7b5d3b9
Compare
bergundy
force-pushed
the
bergundy/nexus-operation-from-workflow
branch
from
7b5d3b9
to
dcfa8cf
Compare
|
@cretz @Quinn-With-Two-Ns anything preventing your approval? |
* Add test environment support for Nexus Operations * Change client to not allow any direct calls
## What was changed - Added the `temporalnexus` package and implemented the handler side for Nexus, including registering and dispatching Nexus Operations. - Added the ability to execute Nexus Operations from a workflow. - Added basic support for running Nexus Operations in the test environment. - Added memoizing to `worker.Start()` to return consistent errors to callers and avoid rerunning the function unnecessarily. - Updated the integration test's dev server to run CLI `0.14.0-nexus.0` which includes server `1.25.0-rc.0`. See the [proposal](https://github.com/temporalio/proposals/blob/b72c49b0c2278e916265b00a49638006f8fce469/nexus/sdk-go.md) for more information. Most of this code has been reviewed already in #1466, #1473, and #1475, which are all squashed in the first commit.
What was changed
Added the ability to execute nexus operations from a workflow.
This PR is stacked on top of #1466.
Checklist