Skip to content

feat: Add AsyncExecutor in MCPTool, ensure MCPTool works in hayhooks #1643

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

Merged
merged 1 commit into from
Apr 11, 2025

Conversation

vblagoje
Copy link
Member

@vblagoje vblagoje commented Apr 11, 2025

Why:

To simplify and standardize the use of MCPTool across different deployment contexts—whether in standalone scripts, sync/async environments, or Hayhooks-based REST deployments. This is achieved by introducing AsyncExecutor and using it for MCPTool invocation.

What:

  • Introduced AsyncExecutor: a utility for executing async code in sync contexts, enabling cleaner and safer integration of MCPTool.
  • Added PipelineWrapper example: a concrete examples/hayhooks/pipeline_wrapper.py of deploying MCPTool in a Haystack pipeline using Hayhooks, exposing it as a REST API.
  • Enhanced client-server communication and tool invocation with improved logging for easier debugging and observability.

How it can be used:

Example hayhooks pipeline wrapper of a time tool in Hayhooks:

class PipelineWrapper(BasePipelineWrapper):
    def setup(self) -> None:
        time_tool = MCPTool(
            name="get_current_time",
            server_info=StdioServerInfo(command="uvx", args=["mcp-server-time", "--local-timezone=Europe/Berlin"]),
        )
        self.pipeline.add_component("llm", OpenAIChatGenerator(model="gpt-4o-mini", tools=[time_tool]))

Example invocation:

curl -X POST 'http://localhost:1416/time_pipeline/run' \
     -H 'accept: application/json' \
     -H 'Content-Type: application/json' \
     -d '{"query":"What is the time in San Francisco? Be brief"}'

How it was tested:

  • Updated unit tests
  • Validated that the REST deployment works end-to-end with the time query example, see examples/hayhooks/pipeline_wrapper.py
  • Validated all existing examples continue to work as well

Notes for the reviewer:

  • Focus on AsyncExecutor and how it simplifies coroutine management across different environments.
  • Review the logging enhancements which improve transparency during tool execution and aid in debugging issues across async boundaries.

@github-actions github-actions bot added integration:mcp type:documentation Improvements or additions to documentation labels Apr 11, 2025
@vblagoje vblagoje requested a review from mpangrazzi April 11, 2025 10:16
@vblagoje vblagoje marked this pull request as ready for review April 11, 2025 10:16
@vblagoje vblagoje requested a review from a team as a code owner April 11, 2025 10:16
@vblagoje vblagoje requested review from davidsbatista and removed request for a team April 11, 2025 10:16
Copy link
Contributor

@mpangrazzi mpangrazzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll approve as discussed!

I'll leave here some notes for next iterations:

  • The main need for this in Hayhooks context is to have a reliable way to run async code (MCP related) in sync environments (PipelineWrapper), which in turn runs in async environment (FastAPI). The main problem is that MCP SDK seems to be not designed to be run on another event loop context (FastAPI), especially if wrapped in sync code.
  • We'll need to implement async versions of setup / run_api in Hayhooks environment, which will ease the dealing with existing event loops.
  • The singleton pattern used in AsyncExecutor to ensure only one executor exists in the application can be a bottleneck: when multiple concurrent requests call run(), they queue up waiting for the single executor thread. A better approach should be to call directly async ainvoke() from an async run_api().

@vblagoje
Copy link
Member Author

vblagoje commented Apr 11, 2025

I'll approve as discussed!

I'll leave here some notes for next iterations:

  • The main need for this in Hayhooks context is to have a reliable way to run async code (MCP related) in sync environments (PipelineWrapper), which in turn runs in async environment (FastAPI). The main problem is that MCP SDK seems to be not designed to be run on another event loop context (FastAPI), especially if wrapped in sync code.
  • We'll need to implement async versions of setup / run_api in Hayhooks environment, which will ease the dealing with existing event loops.
  • The singleton pattern used in AsyncExecutor to ensure only one executor exists in the application can be a bottleneck: when multiple concurrent requests call run(), they queue up waiting for the single executor thread. A better approach should be to call directly async ainvoke() from an async run_api().

In full agreement - couldn't have said it better than this - thanks @mpangrazzi

@vblagoje vblagoje merged commit 2ea5940 into main Apr 11, 2025
10 checks passed
@vblagoje vblagoje deleted the mcp_tool_async_executor branch April 11, 2025 12:07
@vblagoje vblagoje mentioned this pull request Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:mcp type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with MCPTool demo
2 participants