add RunTask.workflow support #896
Conversation
|
Great @treblereel I was working on |
|
@treblereel can you please update the description and link the issue? |
| protected CompletableFuture<WorkflowModel> internalExecute( | ||
| WorkflowContext workflowContext, TaskContext taskContext) { | ||
| if (taskContext.task() != null && taskContext.task() instanceof RunTask runTask) { | ||
| RunWorkflow runWorkflow = runTask.getRun().getRunWorkflow(); |
There was a problem hiding this comment.
This can be null since getRun() will have scripts, shell, container:
private RunTaskConfiguration value;
private RunContainer runContainer;
private RunScript runScript;
private RunShell runShell;
private RunWorkflow runWorkflow;There was a problem hiding this comment.
@ricardozanini I agree this could be revisited later, but for now, the scope of this executor is limited to run.workflow Once additional Run* types are introduced, we can generalize the logic and handle nulls consistently across all cases
There was a problem hiding this comment.
I think this should be done first, not later.
There was a problem hiding this comment.
I think RunTaskExecutor should mimic CallTaskExecutor.
so rather than directly instantiating RunWorkflowExceutor, load the four different executor using serviceloader.
The reason is that the other run task will require additional dependencies and should be located in different modules (like HTTP or OpenAPI)
So, even if running workflow does not need additional dependencies, it should be loaded through serivce loading mechanism
| SubflowConfiguration subflowConfiguration = runWorkflow.getWorkflow(); | ||
| String name = subflowConfiguration.getName(); | ||
| String version = subflowConfiguration.getVersion(); | ||
| String namespace = subflowConfiguration.getNamespace(); | ||
| for (Map.Entry<WorkflowDefinitionId, WorkflowDefinition> kv : | ||
| application.workflowDefinitions().entrySet()) { | ||
| WorkflowDefinitionId workflowDefinitionId = kv.getKey(); | ||
| if (workflowDefinitionId.name().equals(name) | ||
| && workflowDefinitionId.namespace().equals(namespace) | ||
| && workflowDefinitionId.version().equals(version)) { | ||
| WorkflowDefinition workflowDefinition = kv.getValue(); | ||
| WorkflowApplication workflowApplication = workflowDefinition.application(); | ||
| return workflowApplication | ||
| .workflowDefinition(workflowDefinition.workflow()) | ||
| .instance(taskContext.input().asJavaObject()) | ||
| .start(); | ||
| } | ||
| } |
There was a problem hiding this comment.
WorkflowDefinition identification does not need to be done every time the workflow is executed, most of this code should be executed just once.
| return workflowApplication | ||
| .workflowDefinition(workflowDefinition.workflow()) | ||
| .instance(taskContext.input().asJavaObject()) |
There was a problem hiding this comment.
you already have the workflowDefinition, you do not need to retreive it again from the appliaction.
Also, you can create a WorkflowDefinitionId is a record, the proper way to do this should be
WorkflowDefinition workflowDefinition = application.workflowDefinitions().get(new WorkflowDefinitionId(name, namespace.version));
if (workflowDefinition != null) {
return workflowDefinition.instance(taskContext.input()).start();
} else {
// throw exception indicating the workflow is not defined
}
Also, you do not need to call asJavaObject(), instance should accept a WorkflowModel as per https://github.com/serverlessworkflow/sdk-java/blob/main/impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowModelFactory.java#L72-L74
|
@treblereel I have merged #900, please rebase and continue from there, thanks |
Signed-off-by: Dmitrii Tikhomirov <[email protected]>
treblereel
force-pushed
the
workflow_process
branch
from
23b713d to
fb00e36
Compare
4 participants
Fix #897
This change introduces execution support for sub-workflows within the main workflow definition.
It allows a workflow to invoke another workflow declaratively via the RunTask construct