-
Notifications
You must be signed in to change notification settings - Fork 148
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 type safe API to execute an async update workflow request #2320
Add type safe API to execute an async update workflow request #2320
Conversation
335136f
to
fa4329d
Compare
@dandavison FYI The naming of this API may be relevant to your work on update with start |
The only place that the Java SDK uses "execute" AFAIK is So |
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.
No comment on update options
* @param options update options | ||
* @return WorkflowUpdateHandle that can be used to get the result of the update | ||
*/ | ||
static WorkflowUpdateHandle<Void> executeUpdate( |
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.
IMO this should be called startUpdate
even though I know Java is not that consistent here. I think it's clearer to understand and allows us to offer a real executeUpdate
later if we wanted. I'd also support update
.
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.
Yep seems there is consensus that we should call this startUpdate
so I renamed it.
temporal-sdk/src/main/java/io/temporal/client/WorkflowClient.java
Outdated
Show resolved
Hide resolved
a7f710f
to
af0037b
Compare
Add type safe API to execute an async update workflow request.
Open questions we should resolve:
executeUpdate
acceptable? We could also useupdate
orstartUpdate
the Java SDK is not very consistent hereUpdateOptions
OK or do we want a new type here? Reusing is nice since it reduces the number of types, but has some redundant fields likename
, and type.closes #2181