Add tryGettingSimpleWorkflowResult and tryGettingComplexWorkflowResult #199
Conversation
Codecov Report
@@ Coverage Diff @@
## main #199 +/- ##
============================================
+ Coverage 71.49% 72.05% +0.55%
- Complexity 376 388 +12
============================================
Files 60 60
Lines 1575 1585 +10
Branches 139 140 +1
============================================
+ Hits 1126 1142 +16
+ Misses 379 372 -7
- Partials 70 71 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| * @param workflowRunId optional, can be empty | ||
| * @return a list of the state output for completion states. User code will figure how to use ObjectEncoder to decode the output | ||
| */ | ||
| public List<StateCompletionOutput> getComplexWorkflowResultWithoutWait( |
There was a problem hiding this comment.
I personally want to avoid using WithoutWait because it looks too similar to WithWait at quick glance...
If you look at
- getComplexWorkflowResultWithoutWait
- getComplexWorkflowResultWithWait
Theoutis too minor for our human eyes.
Though it looks accurate in grammar 😮💨
Maybe just call tryGettingComplexWorkflowResult -- it seems like Temporal is going to have this name as well:
temporalio/features#314
There was a problem hiding this comment.
Good call, yeah when I was writting the codes I also needed to pay attention on the names...
| @@ -209,6 +306,33 @@ public <T> T getSimpleWorkflowResultWithWait( | |||
| return clientOptions.getObjectEncoder().decode(output.getCompletedStateOutput(), valueClass); | |||
| } | |||
|
|
|||
| private WorkflowGetResponse getWorkflowResponse( | |||
There was a problem hiding this comment.
Nit: to me this should be called getWorkflowResult, and the other one could be something like getWorkflowResultAndDecode
There was a problem hiding this comment.
I used the plural form getWorkflowResults because the return values is from workflowGetResponse.getResults().
zklgame
changed the title
| * In some cases, a workflow may have more than one completion states | ||
| * For most cases, a workflow only has one result(one completion state). | ||
| * Use this API to retrieve the output of the state. | ||
| * If the workflow is still running, return null. |
There was a problem hiding this comment.
Returnning null will be a problem if some workflow actually return null as result. There will be hard to differentiate the null being results or workflow uncompleted.
| throw IwfHttpException.fromFeignException(clientOptions.getObjectEncoder(), exp); | ||
| } | ||
|
|
||
| if (withWait && workflowGetResponse.getWorkflowStatus() != WorkflowStatus.COMPLETED) { |
There was a problem hiding this comment.
so maybe just throw uncompletedException here when withWait==false and workflow is not completed
zklgame
force-pushed
the
zklgame/issue_85
branch
from
14be0d1
to
acea5ea
Compare
| * Use this API to retrieve the output of the state | ||
| * For most cases, a workflow only has one result(one completion state). | ||
| * Use this API to retrieve the output of the state with waiting for the workflow to complete. | ||
| * If the workflow is not COMPLETED, throw the {@link WorkflowUncompletedException}. |
There was a problem hiding this comment.
Actually this is throwing a different exception today: see #202 we may fix that in the future (we can't change this today because it will be a breaking change for production 😮💨 )
issue: #85