Re-factor `WorkflowHandler.run_step()` so user manually emits Event to start next step in worfklow #16277

nerdai · 2024-09-29T03:14:01Z

Description

Currently run_step() is able to run a Workflow steps one at a time, however, it doesn't really provide the user the ability to control the step-by-step execution. In other words, the user isn't able to control when a step ends and thus when to trigger to the next event.

Acceptance Criteria:

The user is able to end the current step by emitting the appropriate event, which then kicks off the next step.

Current Design:

handler = workflow.run(stepwise=True)
while not handler.is_done():
    ev = await handler.run_step()
    handler.ctx.send_event(ev)

result = await handler
print(result)

NOTE:

Currently this breaks the already deprecated Workflow.run_step since this invokes _start(stepwise=True) which has been modified in this re-factor.

Type of Change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Updated relevant unit/integration tests
I stared at the code and made sure it makes sense

nerdai · 2024-09-30T13:55:38Z

llama-index-core/tests/workflow/test_workflow.py

-@pytest.mark.asyncio()
-async def test_deprecated_workflow_run_step(workflow):
-    workflow._verbose = True
-
-    # First step
-    result = await workflow.run_step()
-    assert result is None
-    assert not workflow.is_done()
-
-    # Second step
-    result = await workflow.run_step()
-    assert result is None
-    assert not workflow.is_done()
-
-    # Final step
-    result = await workflow.run_step()
-    assert not workflow.is_done()
-    assert result is None
-
-    # Cleanup step
-    result = await workflow.run_step()
-    assert result == "Workflow completed"
-    assert workflow.is_done()


I think we should probably delete this test and delete Workflow.run_step() now? To support this with the re-factor then we could either:

Maintain an older version of _start() that doesn't contain the new stepwise logic and point this Workflow.run_step() to it

Update the Worfklow.run_step() to have a similar refactor to WorkflowHandler.run_step().

Maybe we should just delete run_step() on the workflow object? (Or at least, delete the implementation and point users towards the updated syntax?)

I think we should just delete it at this point tbh. I think if we point it to the updated syntax means that we're changing how this Workflow.run_step() behaves and not sure if we should do that since its already deprecated.

If you and @masci are fine with updating the logic of this and still marking it as deprecated then I'd be happy to adjust accordingly.

Alright, I deleted Workflow.run_step. I don't think it was worth it to update to point to new code as it would change the functionality of this already deprecated method drastically. As such, keeping it would not really provide any benefits to users who were still using this deprecated method.

logan-markewich · 2024-09-30T16:25:03Z

llama-index-core/llama_index/core/workflow/handler.py

-                retval = self.ctx.get_result()
+                    # Check if we're done
+                    if not t.done():
+                        continue


hmm, is this check actually needed? If a StopEvent is emitted, we don't care if other tasks are still running right?

oh yes, good point.

So, I had to modify this slightly as it wasn't handling any errors raised in a step correctly.

The changes make sense!

logan-markewich

This looks pretty good to me!

With this

users ensure step-by-step runtime
can modify/edit events before emitting them, or even emit more events on the fly
can modify the context state in between steps

…o start next step in worfklow (run-llama#16277) * initial version of stepwise working * remove stepwise deprecated test * remove print * better naming * add docstring to WorkflowHandler * revert Workflow.run_step * fix check for when Workflow is done or if step raised error * fix error handling * fix error handling * update docs * delete Workflow.run_step

nerdai linked an issue Sep 29, 2024 that may be closed by this pull request

As a user, I want to be able to control when the current step is completed by being able to trigger the start of the next step. (Re-factor run_step()) #16244

Closed

nerdai changed the title ~~[WIP] Nerdai/user emit event to start next step in worfklow~~ [WIP] Re-factor WorkflowHandler.run_step() so user manually emits Event to start next step in worfklow Sep 29, 2024

nerdai commented Sep 30, 2024

View reviewed changes

nerdai added 5 commits September 30, 2024 10:27

initial version of stepwise working

756732f

remove stepwise deprecated test

c91b145

remove print

e80e74f

better naming

ceef62d

add docstring to WorkflowHandler

fea868a

nerdai force-pushed the nerdai/user-emit-event-to-start-next-step-in-worfklow branch from d438030 to fea868a Compare September 30, 2024 14:27

nerdai marked this pull request as ready for review September 30, 2024 14:28

dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 30, 2024

nerdai changed the title ~~[WIP] Re-factor WorkflowHandler.run_step() so user manually emits Event to start next step in worfklow~~ Re-factor WorkflowHandler.run_step() so user manually emits Event to start next step in worfklow Sep 30, 2024

logan-markewich requested review from masci and logan-markewich September 30, 2024 16:17

logan-markewich reviewed Sep 30, 2024

View reviewed changes

revert Workflow.run_step

38ba385

logan-markewich approved these changes Sep 30, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 30, 2024

nerdai added 5 commits September 30, 2024 13:33

fix check for when Workflow is done or if step raised error

ef875e8

fix error handling

1588bf9

fix error handling

be481de

update docs

35a9edb

delete Workflow.run_step

d886c49

nerdai merged commit 1024574 into main Oct 1, 2024
10 checks passed

nerdai deleted the nerdai/user-emit-event-to-start-next-step-in-worfklow branch October 1, 2024 13:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-factor `WorkflowHandler.run_step()` so user manually emits Event to start next step in worfklow #16277

Re-factor `WorkflowHandler.run_step()` so user manually emits Event to start next step in worfklow #16277

nerdai commented Sep 29, 2024 •

edited

Loading

nerdai Sep 30, 2024

logan-markewich Sep 30, 2024

nerdai Oct 1, 2024

nerdai Oct 1, 2024 •

edited

Loading

logan-markewich Sep 30, 2024

nerdai Sep 30, 2024

nerdai Sep 30, 2024

logan-markewich Sep 30, 2024

logan-markewich left a comment

Re-factor WorkflowHandler.run_step() so user manually emits Event to start next step in worfklow #16277

Re-factor WorkflowHandler.run_step() so user manually emits Event to start next step in worfklow #16277

Conversation

nerdai commented Sep 29, 2024 • edited Loading

Description

Type of Change

How Has This Been Tested?

nerdai Sep 30, 2024

Choose a reason for hiding this comment

logan-markewich Sep 30, 2024

Choose a reason for hiding this comment

nerdai Oct 1, 2024

Choose a reason for hiding this comment

nerdai Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

logan-markewich Sep 30, 2024

Choose a reason for hiding this comment

nerdai Sep 30, 2024

Choose a reason for hiding this comment

nerdai Sep 30, 2024

Choose a reason for hiding this comment

logan-markewich Sep 30, 2024

Choose a reason for hiding this comment

logan-markewich left a comment

Choose a reason for hiding this comment

Re-factor `WorkflowHandler.run_step()` so user manually emits Event to start next step in worfklow #16277

Re-factor `WorkflowHandler.run_step()` so user manually emits Event to start next step in worfklow #16277

nerdai commented Sep 29, 2024 •

edited

Loading

nerdai Oct 1, 2024 •

edited

Loading