-
Notifications
You must be signed in to change notification settings - Fork 627
Python: [BREAKING] consolidate workflow run APIs #1723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR consolidates workflow execution APIs by removing 4 methods (run_from_checkpoint, run_stream_from_checkpoint, send_responses, send_responses_streaming) and enhancing the existing run() and run_stream() methods to handle all execution scenarios through optional parameters. The change introduces runtime checkpoint storage configuration, allowing checkpointing to be enabled or overridden at execution time rather than only at build time.
Key Changes:
- Unified
run()andrun_stream()methods now acceptmessage,checkpoint_id,checkpoint_storage, andresponsesparameters - Runtime checkpoint storage can override build-time configuration
- All sample code and tests updated to use the new unified API
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/packages/core/agent_framework/_workflows/_workflow.py | Consolidated 6 execution methods into 2 unified methods with enhanced parameter handling and runtime checkpoint storage support |
| python/packages/core/agent_framework/_workflows/_runner_context.py | Added runtime checkpoint storage override capability with set_runtime_checkpoint_storage() and clear_runtime_checkpoint_storage() methods |
| python/packages/core/agent_framework/_workflows/_magentic.py | Removed delegating methods that are now handled by base Workflow class |
| python/packages/core/agent_framework/_workflows/_workflow_executor.py | Updated to use run() with responses parameter |
| python/packages/core/agent_framework/_workflows/_agent.py | Updated to use run_stream() with responses parameter |
| python/packages/core/tests/workflow/test_workflow.py | Updated tests to use new API and added runtime checkpoint configuration tests |
| python/packages/core/tests/workflow/test_sequential.py | Added runtime checkpoint tests for sequential workflows |
| python/packages/core/tests/workflow/test_magentic.py | Updated tests and added runtime checkpoint configuration tests |
| python/packages/core/tests/workflow/test_sub_workflow.py | Updated to use run() with responses parameter |
| python/packages/core/tests/workflow/test_handoff.py | Updated to use run_stream() with responses parameter |
| python/packages/core/tests/workflow/test_group_chat.py | Added runtime checkpoint configuration tests |
| python/packages/core/tests/workflow/test_concurrent.py | Added runtime checkpoint configuration tests |
| python/packages/core/tests/workflow/test_checkpoint_validation.py | Updated to use run_stream() with checkpoint_id parameter |
| python/samples/* | Updated all sample code to use new unified API |
|
I am wondering if it is possible to have this PR reviewed and merged before this one because there is a major refactoring for the request & response pattern. That PR should be ready by the end of today. |
| from this checkpoint instead of starting fresh. When resuming, checkpoint_storage | ||
| must be provided (either at build time or runtime) to load the checkpoint. | ||
| checkpoint_storage: Runtime checkpoint storage with two behaviors: | ||
| - With checkpoint_id: Used to load and restore the specified checkpoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This usage will also result in enabling checkpointing for this run, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the options are:
- Enable checkpointing on the builder - will apply to every workflow run for that checkpoint instance (can still be useful)
- Checkpointing is now allowed (and enabled) on a per-run basis, and will take precedence over the workflow level config.
| WorkflowEvent: Events generated during workflow execution. RequestInfoEvent instances contain | ||
| request_id fields that must be used as keys in the responses dictionary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why call out RequestInfoEvent here when there are a lot of WorkflowEvents. Maybe this could link to a readme or something where they are all documented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can remove the callout to RequestInfoEvent - especially because it'll be gone in Tao's PR. The goal is to get Tao's HIL refactor PR in first, then I'll bring in changes here and clean this up.
|
Waiting for #1466 to go in first, then will bring in changes to this PR. |
Motivation and Context
Summary
Consolidates workflow execution APIs from 6 methods into 2 unified methods with enhanced checkpoint storage capabilities.
Breaking Changes
run_from_checkpoint(),run_stream_from_checkpoint(),send_responses(),send_responses_streaming()run()orrun_stream()with optional parametersNew Capabilities
checkpoint_storageparameter to enable/override checkpointing at runtimemessage,checkpoint_id,checkpoint_storage,responsessupport all execution scenariosAPI Changes
Implementation
Updated InProcRunnerContext to support runtime checkpoint storage override
Updated all internal consumers: WorkflowAgent, WorkflowExecutor, MagenticWorkflow
Updated all tests to use new API
Added comprehensive test coverage for runtime checkpoint scenarios across all workflow builders
TODO: will update Learn site docs.
Description
run_stream()#1536Contribution Checklist