conductor-sdk / conductor-python

Conductor OSS SDK for Python programming language
Apache License 2.0
52 stars 26 forks source link

Fix DoWhileTask/LoopTask with several subtasks #220

Closed aircey closed 7 months ago

aircey commented 7 months ago

Hello,

An exception is raised when adding a DoWhileTask or a LoopTask that contains a list of tasks to a workflow.

workflow = ConductorWorkflow(...)
task1 = SimpleTask(...)
task2 = SimpleTask(...)

dowhile_single = DoWhileTask(
    task_ref_name='dowhile_single',
    termination_condition="$.dowhile_single['iteration'] < 2",
    tasks = task1
)
workflow.add(dowhile_single)
# => OK

dowhile_multiple = DoWhileTask(
    task_ref_name='dowhile_multiple',
    termination_condition="$.dowhile_multiple['iteration'] < 2",
    tasks = [ task1, task2 ]
)
workflow.add(dowhile_multiple)
# => KO
# File "/usr/local/lib/python3.11/site-packages/conductor/client/workflow/task/do_while_task.py", line 32, in to_workflow_task
# workflow.loop_over = get_task_interface_list_as_workflow_task_list(
#                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
# TypeError: conductor.client.workflow.task.task.get_task_interface_list_as_workflow_task_list() argument after * must be an iterable, not SimpleTask

It seems this bug has been introduced along this commit @gardusig https://github.com/conductor-sdk/conductor-python/commit/5b5bf04a5408b799111fc071b7fb62fa2104ac80#diff-6561dd5f4d67f7b315998c89eafc7cee302c3c76e800ee8538b9f7b0594cffdeR9

The signature of task.get_task_interface_list_as_workflow_task_list() changed:

# from:
def get_task_interface_list_as_workflow_task_list(tasks: List[TaskInterface]) -> List[WorkflowTask]:
# to:
def get_task_interface_list_as_workflow_task_list(*tasks: TaskInterface) -> List[WorkflowTask]:

... but has not been updated accordingly in the caller class DoWhileTask.

My fix proposal applies the same transformation as SwitchTask: https://github.com/conductor-sdk/conductor-python/blob/6b8744d394399bd0d535ac12e412e03c54bee0ef/src/conductor/client/workflow/task/switch_task.py#L26-L31

gardusig commented 7 months ago

Hi @romaincolombo,

I no longer work at the company who manages this repo, so can't help much with this. Maybe ping @v1r3n or @coderabhigupta to take a look at it.