Python: fix: use workflow factory to avoid RuntimeError under parallel requests#4772
Python: fix: use workflow factory to avoid RuntimeError under parallel requests#4772LEDazzio01 wants to merge 2 commits intomicrosoft:mainfrom
Conversation
…ts (microsoft#4766) Pass a factory lambda to `from_agent_framework()` instead of a pre-built agent instance so each hosted request gets a fresh workflow. Previously, the single shared workflow would raise `RuntimeError: Workflow is already running. Concurrent executions are not allowed.` when parallel requests arrived.
There was a problem hiding this comment.
Pull request overview
This PR fixes a concurrency issue in the Python hosted-agent workflow sample by ensuring each incoming hosted request gets a fresh Workflow instance rather than reusing a shared one (which can raise RuntimeError: Workflow is already running... under parallel load).
Changes:
- Update the hosted-agent sample to pass a workflow factory callable into
from_agent_framework(...)so the workflow is constructed per request. - Keep the writer/reviewer agent creation separate from workflow instantiation.
You can also share your feedback on Copilot code review. Take the survey.
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py
Outdated
Show resolved
Hide resolved
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py
Outdated
Show resolved
Hide resolved
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py
Show resolved
Hide resolved
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py
Show resolved
Hide resolved
|
Thanks for the review, Copilot! All 4 comments are about pre-existing code in the sample rather than the factory-lambda fix introduced in this PR. I'll keep the scope of this PR focused on the concurrency fix (#4766). That said, these are valid observations about the original sample — particularly the |
moonbox3
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 89%
✗ Correctness
The new sample has two correctness bugs: (1) it calls
.run_async()on the object returned byfrom_agent_framework(), but all existing samples use.run()—run_asynclikely does not exist and will raiseAttributeErrorat runtime; (2) it passes a lambda factory tofrom_agent_framework()instead of an agent instance, diverging from every other hosted-agent sample which passes the agent directly. Additionally, the entiremain()is needlessly async withasyncio.run()when the established pattern is a synchronousmain()calling.run().
✓ Security Reliability
New sample introduces a custom credential-selection mechanism using the MSI_ENDPOINT environment variable instead of the established DefaultAzureCredential pattern used by all other hosted-agent samples. This hand-rolled approach is less robust (ignores IDENTITY_ENDPOINT used in newer Azure hosting, workload identity, and other credential types) and reduces reliability. No hard security vulnerabilities were found, but the credential deviation is a reliability concern worth addressing.
✓ Test Coverage
This diff adds a new end-to-end sample file demonstrating a writer-reviewer multi-agent workflow. The samples directory has no unit tests for any existing samples, so the absence of tests here is consistent with project convention. The file is a runnable demo requiring live Azure credentials, not library code that would warrant unit tests. No test coverage concerns specific to this change.
✗ Design Approach
This new sample hand-rolls credential selection by sniffing the MSI_ENDPOINT environment variable, while every other sample in the repo (~20+) uses DefaultAzureCredential — the Azure SDK's built-in credential chain that already handles ManagedIdentity, AzureCLI, environment variables, and more. The custom get_credential() function is a fragile reimplementation of existing SDK behavior: MSI_ENDPOINT is specific to older Azure App Service; newer compute (Container Apps, Functions v4, AKS workload identity) uses IDENTITY_ENDPOINT or other mechanisms. Since samples teach patterns, shipping this trains users to re-invent a credential chain that the SDK already provides correctly.
Flagged Issues
-
.run_async()does not appear to exist on the object returned byfrom_agent_framework(). All other hosted-agent samples use.run(). This will raiseAttributeErrorat runtime. -
from_agent_framework()is called with a lambda (factory function) instead of an agent instance. All other hosted-agent samples pass the agent directly (e.g.,from_agent_framework(workflow_agent).run()). Passing a lambda may cause a type error or unexpected behavior at runtime. - Hand-rolled credential selection via
MSI_ENDPOINTcheck is a fragile reimplementation ofDefaultAzureCredential.MSI_ENDPOINTonly exists on older App Service SKUs; newer Azure compute (Container Apps, Functions v4, AKS workload identity) usesIDENTITY_ENDPOINTor federation. Every other sample in this repo usesDefaultAzureCredential— this sample should too.
Suggestions
- Follow the established pattern: make
main()synchronous, build the agents/workflow inline, and callfrom_agent_framework(agent).run()directly — seeagents_in_workflow/main.pyfor reference. - If
AzureOpenAIResponsesClientrequires async context-manager cleanup, consider whether the credential and client can be created withoutasync with, or restructure so the agent is created before callingfrom_agent_framework. - Consider validating that
PROJECT_ENDPOINTis set before passing it toAzureOpenAIResponsesClient, to provide a clear error message instead of silently falling through to an alternative initialization path.
Automated review by moonbox3's agents
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py
Outdated
Show resolved
Hide resolved
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py
Outdated
Show resolved
Hide resolved
|
@LEDazzio01 once comments are addressed and no longer relevant, it's really helpful for us if you can please resolve them. Thanks. |
- Switch to sync DefaultAzureCredential (matches all other samples) - Use from_agent_framework(agent).run() instead of .run_async() - Remove unnecessary async/asyncio patterns - Change load_dotenv(override=True) to load_dotenv() Addresses review feedback from @moonbox3 and Copilot.
|
Thanks for the review @moonbox3! I've addressed all the feedback:
Re: Copilot's env var naming comment ( I'll resolve the comment threads now. Thanks! |
eb740d6 to
49153de
Compare
Summary
Fixes #4766
The hosted agent sample at
writer_reviewer_agents_in_workflow/main.pycreates a single workflow agent and passes it directly tofrom_agent_framework(). When the hosted endpoint receives parallel requests, the sharedWorkflowinstance attempts to run concurrently, raising:Root Cause
from_agent_framework()accepts either a pre-built agent or a factory callable. When given a factory, it creates a fresh agent per request, avoiding shared state.Fix
This ensures each incoming request gets its own
Workflowinstance, eliminating theRuntimeErrorunder concurrent load.Changes
python/samples/05-end-to-end/hosted_agents/writer_reviewer_agents_in_workflow/main.py: Pass a factory lambda tofrom_agent_framework()instead of a pre-built agent instanceContribution Checklist