-
Notifications
You must be signed in to change notification settings - Fork 220
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
Nexus: Fix link not being attached to a workflow started via a Handler function #1659
Conversation
temporalnexus/operation.go
Outdated
@@ -262,11 +252,14 @@ type WorkflowHandle[T any] interface { | |||
ID() string | |||
// ID is the workflow's run ID. | |||
RunID() string | |||
// Link to the WorkflowExecutionStarted event of the workflow represented by this handle. | |||
Link() nexus.Link |
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.
hm why does this need to be on the interface? Are users expected to use this method?
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 doesn't but I don't think it hurts.
Another approach would be to put this on the struct implementation and have an unexported method on the interface to ensure users don't try to implement it. This is important since the handler's return value is the interface.
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.
I'll change to that alternative.
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.
Done.
I understand if we can't write an end to end test for this, but what is stopping us from unit testing this change? |
We'll add integration tests soon. server |
Not testing this since server 1.25.0 didn't get support for links.