Client ActivityType [Go] #179
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stephanos
force-pushed
the
kitchensink-client-activity
branch
4 times, most recently
from
dc13182 to
3bb375e
Compare
stephanos
force-pushed
the
kitchensink-client-activity
branch
12 times, most recently
from
4e43179 to
3a55098
Compare
stephanos
force-pushed
the
kitchensink-client-activity
branch
12 times, most recently
from
5e0a5ce to
6f1bd89
Compare
stephanos
commented
Aug 18, 2025
| } | ||
| }) | ||
|
|
||
| // Handle initial set. |
Collaborator
Author
There was a problem hiding this comment.
Needs to be after signal handler is registered.
Merged
stephanos
force-pushed
the
kitchensink-client-activity
branch
12 times, most recently
from
3c2b7c4 to
92d73d3
Compare
stephanos
force-pushed
the
kitchensink-client-activity
branch
from
92d73d3 to
2cd1d99
Compare
stephanos
force-pushed
the
kitchensink-client-activity
branch
3 times, most recently
from
343d863 to
84014ba
Compare
Sushisource
approved these changes
Aug 26, 2025
Member
Sushisource
left a comment
Sushisource
left a comment
There was a problem hiding this comment.
Looking good to me. The C# bit might need some verification.
| let client_sequence = ClientSequence { | ||
| action_sets: vec![ClientActionSet { | ||
| actions: vec![u.arbitrary()?], | ||
| concurrent: false, |
Member
There was a problem hiding this comment.
Should this be allowed to be true? Seemingly the support is already there?
Collaborator
Author
There was a problem hiding this comment.
The support for Go is there but the support for the other SDKs isn't there yet. #188 will only add support for non-current client activity there.
Comment on lines
150
to
151
| await LaunchActivity(execActivity, tokenSrc); | ||
| await HandleAwaitableChoiceAsync(Task.FromResult<object?>(null), tokenSrc, execActivity.AwaitableChoice); |
Member
There was a problem hiding this comment.
Hmmm... not sure that's right. Probably we need to just fix HandleAwaitableChoice to not eat the exception in this case (though not sure why it is in the first place).
Otherwise I would expect something like var actAwait = LaunchActivity(execActivity, tokenSrc); and then passing that to HandleAwaitableChoiceAsync
stephanos
added a commit
that referenced
this pull request
Aug 27, 2025
<!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed <!-- Describe what has changed in this PR --> Prelude for #179: - adds partial history matching (when full matching won't work) - scopes build logs to test run - runs kitchen sink tests in parallel - fixes data race - adds new end-to-end ThroughputStress test ## Why? <!-- Tell your future self why have you made these changes --> Improve test experience. ## Checklist <!--- add/delete as needed ---> 1. Closes <!-- add issue number here --> 2. How was this tested: <!--- Please describe how you tested your changes/how we can test them --> 3. Any docs updates needed? <!--- update README if applicable or point out where to update docs.temporal.io -->
stephanos
force-pushed
the
kitchensink-client-activity
branch
from
84014ba to
e1ff4a5
Compare
stephanos
added a commit
that referenced
this pull request
Sep 4, 2025
<!--- Note to EXTERNAL Contributors --> <!-- Thanks for opening a PR! If it is a significant code change, please **make sure there is an open issue** for this. We work best with you when we have accepted the idea first before you code. --> <!--- For ALL Contributors 👇 --> ## What was changed <!-- Describe what has changed in this PR --> Follow-up to #179 to include other SDKs. Adds new "Client" ActivityType to execute client actions (e.g. Update, Signal) from a workflow activity to TS, .NET and Python. The client action executor implementations were all modeled after the Go one. --- ℹ️ Concurrent execution of the client actions is out of scope for this PR. 🫘 I couldn't get Java to work; it's out of scope for this PR now. Sorry.⚠️ The ClientActionExecutors were mostly AI generated - with lots of manual pruning and fixing - but I can't quite vouch for them being 100% idiomatic. ## Why? <!-- Tell your future self why have you made these changes --> Narrow kitchensink support gap to Go. ## Checklist <!--- add/delete as needed ---> 1. Closes <!-- add issue number here --> 2. How was this tested: <!--- Please describe how you tested your changes/how we can test them --> 3. Any docs updates needed? <!--- update README if applicable or point out where to update docs.temporal.io -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ℹ️ This PR includes the #189 PR; you can just look at the "Client ActivityType" commit for review.
What was changed
Adds new "Client" ActivityType to execute client actions (e.g. Update, Signal) from a workflow activity.
Why?
Checklist
Closes
How was this tested: