Skip to content

Nexus: use links from StartWorkflowExecutionResponse if present #963

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 14, 2025
Merged

Nexus: use links from StartWorkflowExecutionResponse if present #963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions temporalio/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,12 @@ def __init__(
result_run_id: Optional[str] = None,
first_execution_run_id: Optional[str] = None,
result_type: Optional[Type] = None,
start_workflow_response: Optional[
Union[
temporalio.api.workflowservice.v1.StartWorkflowExecutionResponse,
temporalio.api.workflowservice.v1.SignalWithStartWorkflowExecutionResponse,
]
] = None,
) -> None:
"""Create workflow handle."""
self._client = client
Expand All @@ -1550,6 +1556,7 @@ def __init__(
self._result_run_id = result_run_id
self._first_execution_run_id = first_execution_run_id
self._result_type = result_type
self._start_workflow_response = start_workflow_response
self.__temporal_eagerly_started = False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to make the links from the StartWorkflow response available to code that is in possession of a WorkflowHandle. This PR is currently proposing to attach the entire reponse proto to the workflow handle, but only exposed via an underscore-prefixed attribute, so that it can be used by SDK code but it is not advertised as a stable public API for now.

Copy link
Member

@cretz cretz Jul 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Mentioned off-PR, but will add here too...

This has been an issue in other SDKs. I have proposed that the start caller provide something that can be populated with extra things instead of tacking it on the handle which has more use than just as a start result holder. For instance, have start_workflow accept a mutable StartWorkflowResponseDetails that can have these extra things set on it from the response.

In addition to this, there are actually three valuable user-facing values that can come back from start we should expose: 1) started - whether it actually ended up starting a workflow, which can be false in cases of use-existing conflict policy, 2) status - there is a rare case with update with start where a start workflow succeeds without resulting in a running workflow, and 3) eagerly_started - whether it eagerly started which users may want to know.

We need this across SDKs, it's not a Python only need, and I think we can take the same approach.


@property
Expand Down Expand Up @@ -5832,6 +5839,7 @@ async def start_workflow(
result_run_id=resp.run_id,
first_execution_run_id=first_execution_run_id,
result_type=input.ret_type,
start_workflow_response=resp,
)
setattr(handle, "__temporal_eagerly_started", eagerly_started)
return handle
Expand Down
2 changes: 1 addition & 1 deletion temporalio/nexus/_link_conversion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
LINK_EVENT_TYPE_PARAM_NAME = "eventType"


def workflow_handle_to_workflow_execution_started_event_link(
def workflow_execution_started_event_link_from_workflow_handle(
handle: temporalio.client.WorkflowHandle[Any, Any],
) -> temporalio.api.common.v1.Link.WorkflowEvent:
"""Create a WorkflowEvent link corresponding to a started workflow"""
Expand Down
36 changes: 21 additions & 15 deletions temporalio/nexus/_operation_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from typing_extensions import Concatenate

import temporalio.api.common.v1
import temporalio.api.workflowservice.v1
import temporalio.client
import temporalio.common
from temporalio.nexus import _link_conversion
Expand Down Expand Up @@ -142,25 +143,30 @@ def _get_workflow_event_links(
def _add_outbound_links(
self, workflow_handle: temporalio.client.WorkflowHandle[Any, Any]
):
# If links were not sent in StartWorkflowExecutionResponse then construct them.
Copy link
Member

Choose a reason for hiding this comment

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

Is it valuable to even parse the link? Can we just take any link w/ workflow event on it from the response and set it on outbound?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No because outbound is list[nexusrpc.Link]

Copy link
Member

Choose a reason for hiding this comment

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

I see. So I wonder then what the server links has that makes it better than the one manually built here, but I can understand if the server one should be the canonical one.

wf_event_links: list[temporalio.api.common.v1.Link.WorkflowEvent] = []
try:
link = _link_conversion.workflow_event_to_nexus_link(
_link_conversion.workflow_handle_to_workflow_execution_started_event_link(
workflow_handle
)
if isinstance(
workflow_handle._start_workflow_response,
temporalio.api.workflowservice.v1.StartWorkflowExecutionResponse,
):
if workflow_handle._start_workflow_response.HasField("link"):
if link := workflow_handle._start_workflow_response.link:
if link.HasField("workflow_event"):
wf_event_links.append(link.workflow_event)
if not wf_event_links:
wf_event_links = [
_link_conversion.workflow_execution_started_event_link_from_workflow_handle(
workflow_handle
)
]
self.nexus_context.outbound_links.extend(
_link_conversion.workflow_event_to_nexus_link(link)
for link in wf_event_links
)
except Exception as e:
logger.warning(
f"Failed to create WorkflowExecutionStarted event link for workflow {id}: {e}"
)
else:
self.nexus_context.outbound_links.append(
# TODO(nexus-prerelease): Before, WorkflowRunOperation was generating an EventReference
# link to send back to the caller. Now, it checks if the server returned
# the link in the StartWorkflowExecutionResponse, and if so, send the link
# from the response to the caller. Fallback to generating the link for
# backwards compatibility. PR reference in Go SDK:
# https://github.com/temporalio/sdk-go/pull/1934
link
f"Failed to create WorkflowExecutionStarted event links for workflow {workflow_handle}: {e}"
)
return workflow_handle

Expand Down