Skip to content

Implement GetWorkflowExecutionResult#10173

Open
long-nt-tran wants to merge 5 commits intotemporalio:mainfrom
long-nt-tran:get-workflow-result
Open

Implement GetWorkflowExecutionResult#10173
long-nt-tran wants to merge 5 commits intotemporalio:mainfrom
long-nt-tran:get-workflow-result

Conversation

@long-nt-tran
Copy link
Copy Markdown
Contributor

@long-nt-tran long-nt-tran commented May 4, 2026

What changed?

Server PR for API counterpart temporalio/api#778

This API allows users to get the result of a workflow execution and optionally attach callbacks to be fired when the workflow completes (if the workflow is running). It follows continue-as-new chain up to GetWorkflowExecutionResultMaxCANChainDepth iterations (for now defined to be 100 in dynamic config).

A lot of boilerplate from making a new API (proto + mock generated lines). Business logic to note are in:

  • Frontend handler: service/frontend/workflow_handler.go
  • History handler: service/history/handler.go
  • Actual API Invoke(...) implementation: service/history/api/getworkflowexecutionresult/api.go

Why?

Allows Nexus handler to map to this operation more ergonomically for callers.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

Potential risks

New API -- need more test across repos (load testing, etc...). Gated behind dynamic config.

@long-nt-tran long-nt-tran force-pushed the get-workflow-result branch 10 times, most recently from 71e932b to 9ec54e6 Compare May 6, 2026 21:40
@long-nt-tran long-nt-tran force-pushed the get-workflow-result branch from 9ec54e6 to 807f96b Compare May 7, 2026 22:19
@long-nt-tran long-nt-tran marked this pull request as ready for review May 7, 2026 22:43
@long-nt-tran long-nt-tran requested review from a team as code owners May 7, 2026 22:43
Comment thread tests/nexus_test_base.go
Comment on lines +47 to +48
req.Service = cmp.Or(req.Service, "test-service")
req.Operation = cmp.Or(req.Operation, "test-operation")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opportunistic refactor from nexus_standalone_test.go, but maybe slightly overboard with still keeping these defaults here :) LMK if folks prefer to not have defaults at all

Comment on lines +426 to +434
leafErr := pollResp.GetFailure()
for leafErr.GetCause() != nil {
leafErr = leafErr.GetCause()
}
protorequire.ProtoEqual(s.T(),
&failurepb.Failure{Message: intentionalFailureMsg},
leafErr,
protorequire.IgnoreFields("source", "stack_trace", "encoded_attributes", "application_failure_info"),
)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seemed slightly clunky to me but I wasn't sure there was another way to unwrap and assert on this base error, LMK if there is

Comment thread service/history/api/getworkflowexecutionresult/api.go
Comment thread common/api/metadata.go Outdated

// Walk the CAN chain until we reach the head. At head, we will return the result of the head's workflow run,
// and optionally attach callbacks if hasCallbacks.
for range maxCANChainDepth {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was what I did in my prototype, but I question if this is the right approach to have an arb. limit here. I wonder if we could be smarter but it would require a larger effort. I was thinking if we only support targeting the latest run ID then we don't need to tackle this problem yet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commenting for posterity per offline discussion: will always get result for latest (empty) run id for now to make this simpler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants