break out startUpdate vs executeUpdate
#107
Conversation
johnmastro
changed the title
startUpdate vs executeUpdate
| @@ -139,7 +140,7 @@ import Unsafe.Coerce | |||
| -- WorkflowClient stuff | |||
|
|
|||
| workflowClient | |||
| :: MonadIO m | |||
| :: (MonadIO m) | |||
There was a problem hiding this comment.
i'm not sure if the precommit hook made these formatting changes, or zed (which i took for a test drive here), or what, but i can change it back if anyone hates them
There was a problem hiding this comment.
I tried zed and it did this too. I set "format_on_save": "off" in the config to make it a bit less aggressive about doing this. I don't particularly mind the change though
| @@ -97,8 +97,8 @@ updateWithValidatorThatThrows = provideCallStack do | |||
| registerWorkflow 'updateWithValidatorThatThrows | |||
|
|
|||
|
|
|||
| updateWithValidatorThatSleeps :: Workflow Int | |||
| updateWithValidatorThatSleeps = provideCallStack do | |||
| updateThatSleeps :: Workflow Int | |||
There was a problem hiding this comment.
i happened to notice that the old name was a misnomer (there's no validator, and validation isn't relevant to the tests that use it)
| UpdateLifecycleStageAdmitted -> Update.UPDATE_WORKFLOW_EXECUTION_LIFECYCLE_STAGE_ADMITTED | ||
| UpdateLifecycleStageAccepted -> Update.UPDATE_WORKFLOW_EXECUTION_LIFECYCLE_STAGE_ACCEPTED | ||
| UpdateLifecycleStageCompleted -> Update.UPDATE_WORKFLOW_EXECUTION_LIFECYCLE_STAGE_COMPLETED | ||
| & Update.lifecycleStage .~ Update.UPDATE_WORKFLOW_EXECUTION_LIFECYCLE_STAGE_ACCEPTED |
There was a problem hiding this comment.
I initially thought this was wrong, but I guess you're able to poll later for other lifecycle stages, so 👍🏻
sdk/src/Temporal/Client.hs
Outdated
| or throw an exception if the update failed. | ||
| -} | ||
| waitForUpdateOutcome :: (MonadIO m) => UpdateHandle a -> m a | ||
| waitForUpdateOutcome h = do |
There was a problem hiding this comment.
Can we name this waitUpdateResult to match the waitWorkflowResult function?
There was a problem hiding this comment.
just pushed a commit doing this
sdk/test/IntegrationSpec.hs
Outdated
| ) | ||
| `shouldThrow` \case | ||
| UpdateFailure _ -> True | ||
| _ -> False |
There was a problem hiding this comment.
Unless UpdateFailure has other constructors, these pattern matches are redundant
There was a problem hiding this comment.
ah, right. what i really want to test here is that we threw an UpdateFailure and not, for example, an RpcError.
reading the hspec docs, it sounds like i would want to write a selector for that. is that right?
There was a problem hiding this comment.
Well, you are testing that on line 1599. You just don't need the wildcard match to do that because you've already constrained the type by pattern matching.
There was a problem hiding this comment.
oh, i see. all right, just pushed a commit to remove the redundant branches
refactor
updateinto:startUpdate, which returns anUpdateHandlewithout waiting for the update to complete. but note thatstartUpdatedoes wait for the validator (if any) to complete (i.e. that the update reaches theACCEPTEDstage) so that we can signal anUpdateFailederror for validator failures rather than an "update not found"RpcError(which is what would happen if we tried to poll for the outcome where the validator rejected the update). this is the same thing the typescript sdk does.waitForUpdateOutcome, which, given anUpdateHandle, waits for it to complete and returns the resultexecuteUpdate, which composes those two start the update and wait for its result (the same behavior as the originalupdate)