Adding completion to the Core SDK #35
Conversation
| message PollTaskReq { | ||
| // If true, poll for workflow tasks | ||
| bool workflows = 1; | ||
| // If true, poll for activity tasks |
There was a problem hiding this comment.
I would make it enum to support some future extensions for poll types.
There was a problem hiding this comment.
How are you thinking exactly? The enum can only take on one value, so we really need like a bitmask or something but that doesn't exist in protobuf.
Looks like repeated enum works so I can do that
|
|
||
| // An instruction to the lang sdk to run some workflow code, whether for the first time or from | ||
| // a cached state. | ||
| message WFActivation { |
There was a problem hiding this comment.
We need to add some fields to validate that the cache is not stale. One option is to pass previousEventId and the lastEventId. Another option is to pass some opaque tokens instead.
| // An instruction to the lang sdk to run some workflow code, whether for the first time or from | ||
| // a cached state. | ||
| message WFActivation { | ||
| // Time the activation(s) were requested |
There was a problem hiding this comment.
The comment sounds like the system time of the activation.
But it is a workflow time of the activation.
| /// language SDK's responsibility to call the appropriate code with the provided inputs. | ||
| /// | ||
| /// TODO: Examples | ||
| fn poll_task(&self, task_queue: &str) -> Result<Task>; |
There was a problem hiding this comment.
Does it mean that we are not supporting core polling on multiple queues and language SDK being single threaded?
There was a problem hiding this comment.
It should be possible, that's the whole reason why we pass it as a parameter as opposed to a field during initialization.
| pub trait RespondWorkflowTaskCompletedApi { | ||
| /// Fetch new work. Should block indefinitely if there is no work. | ||
| async fn get_work(&self, task_queue: &str) -> Result<PollWorkflowTaskQueueResponse>; | ||
| async fn complete( |
There was a problem hiding this comment.
The above comment doesn't make sense anymore.
| fn intercept(mut req: Request<()>) -> Result<Request<()>, Status> { | ||
| // TODO convert error | ||
| let metadata = req.metadata_mut(); | ||
| metadata.insert("grpc-timeout", "50000m".parse().unwrap()); |
There was a problem hiding this comment.
The 50 second timeout applies only to long polls. All other requests should have a much shorter (ideally configurable) one.
There was a problem hiding this comment.
We are just testing things out at this point, definitely not a final version.
# Conflicts: # src/lib.rs # tests/integ_tests/poller_test.rs
|
Moved to #36 |
3 participants
This also adds end to end integration test that creates and executes the workflow with a timer.