Cancellable get results #169
Conversation
dbos/workflow.go Outdated
| case <-h.dbosContext.Done(): | ||
| return *new(R), h.dbosContext.Err() |
There was a problem hiding this comment.
@maxdml I'm unable to get to this code path, so not sure if we really need it.
Tried shutting down DBOS server, but it get DB error from h.outcomeChan instead.
Let me know your thoughts
There was a problem hiding this comment.
For this path to kick in, you need to manually Launch and Shutdown the DBOSContext in a test. The sequence would be:
- Launch
- Run blocking workflow (so that the function doesn't return and its result never reaches
outcomeChan) - GetResult with no timeout
- Shutdown DBOSContext
- GetResult should result the error (which should be a context.Context DeadlineExceeded error)
There was a problem hiding this comment.
For this to work, the workflow must signal that it started, which you can achieve by using the test utilities "event" API. See an example here.
There was a problem hiding this comment.
For this case, should it be context.Canceled instead?
There was a problem hiding this comment.
You mean for a test exercising case <-h.dbosContext.Done(): ? You're right, it depends on how the context is cancelled (manually or through a deadline)
There was a problem hiding this comment.
Thanks @PhakornKiong . Left a few comments. Let me know if you have more questions!
dbos/workflow.go Outdated
| case <-h.dbosContext.Done(): | ||
| return *new(R), h.dbosContext.Err() |
There was a problem hiding this comment.
For this path to kick in, you need to manually Launch and Shutdown the DBOSContext in a test. The sequence would be:
- Launch
- Run blocking workflow (so that the function doesn't return and its result never reaches
outcomeChan) - GetResult with no timeout
- Shutdown DBOSContext
- GetResult should result the error (which should be a context.Context DeadlineExceeded error)
dbos/workflow.go Outdated
| case <-h.dbosContext.Done(): | ||
| return *new(R), h.dbosContext.Err() |
There was a problem hiding this comment.
For this to work, the workflow must signal that it started, which you can achieve by using the test utilities "event" API. See an example here.
PhakornKiong force-pushed the cancellable-get-results branch from 8653f7d to 523e6af Compare 
maxdml force-pushed the cancellable-get-results branch from 523e6af to 57a0a17 Compare 
| @maxdml Noticed that there are test like I tired running on local as well as on my forked repo and it went fine. Let me know if there are any other changes that i should make |
2 participants
Resolves #85