Skip to content
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

Add event ID to RequestIDInfo and request ID to CallbackInfo #7491

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rodrigozhou
Copy link
Contributor

What changed?

Add event ID to RequestIDInfo and request ID to CallbackInfo.
Also added RequestIdInfos to DescribeWorkflowExecutionResponse.

Why?

Be able to identify the exact event that attached the callback to the workflow.

How did you test it?

Potential risks

Documentation

Is hotfix candidate?

@rodrigozhou rodrigozhou requested review from yycptt and pdoerner March 19, 2025 05:26
@rodrigozhou rodrigozhou requested a review from a team as a code owner March 19, 2025 05:26
@rodrigozhou rodrigozhou force-pushed the rodrigozhou/request-id-to-event-id branch from 25341e9 to b13745e Compare March 19, 2025 06:02
Copy link
Contributor

@pdoerner pdoerner left a comment

Choose a reason for hiding this comment

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

What happens in the event that the request ID is not populated? It is not strictly required by Nexus. Can we have fallback to link to the WorkflowExecutionStarted event at least?

@@ -342,6 +343,7 @@ func (s *historyCurrentExecutionSuite) newRandomCurrentExecutionRow(
RequestIds: map[string]*persistencespb.RequestIDInfo{
uuid.NewString(): {
EventType: enumspb.EVENT_TYPE_WORKFLOW_EXECUTION_OPTIONS_UPDATED,
EventId: common.FirstEventID,
Copy link
Contributor

Choose a reason for hiding this comment

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

should be common.BufferedEventID?

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