Skip to content

fix: resolve asyncio event loop conflicts in MCP server registration#134

Merged
tisnik merged 1 commit into
lightspeed-core:mainfrom
eranco74:asyncio_conflicts
Jun 26, 2025
Merged

fix: resolve asyncio event loop conflicts in MCP server registration#134
tisnik merged 1 commit into
lightspeed-core:mainfrom
eranco74:asyncio_conflicts

Conversation

@eranco74
Copy link
Copy Markdown
Contributor

Description

Add async/await support for LlamaStackAsLibraryClient to prevent
"Cannot run event loop while another loop is running" RuntimeError.

Changes:
- Introduce register_mcp_servers_async() for async MCP server registration
- Refactor MCP registration logic into separate sync/async helper functions
- Use async client methods (client.async_client) for library clients
to avoid "Cannot run event loop while another loop is running" errors
- Add async tests for library client configuration

The LlamaStackAsLibraryClient requires async initialization, but the previous
sync implementation caused event loop conflicts. This change provides dual
support for both service clients (sync) and library clients (async).

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #
    Fixes RuntimeError when configuration.llama_stack.use_as_library_client = true

  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Copy link
Copy Markdown
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

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

it looks ok, but would like @manstis to test it if possible. @manstis PLEASE

@eranco74 eranco74 force-pushed the asyncio_conflicts branch from 6aee638 to e808906 Compare June 26, 2025 10:58
Add async/await support for LlamaStackAsLibraryClient to prevent
"Cannot run event loop while another loop is running" RuntimeError.

Changes:
- Introduce register_mcp_servers_async() for async MCP server registration
- Refactor MCP registration logic into separate sync/async helper functions
- Use async client methods (client.async_client) for library clients
  to avoid "Cannot run event loop while another loop is running" errors
- Add async tests for library client configuration

The LlamaStackAsLibraryClient requires async initialization, but the previous
sync implementation caused event loop conflicts. This change provides dual
support for both service clients (sync) and library clients (async).

Fixes RuntimeError when configuration.llama_stack.use_as_library_client = true
@eranco74 eranco74 force-pushed the asyncio_conflicts branch from e808906 to ea0c1dd Compare June 26, 2025 11:14
Copy link
Copy Markdown
Contributor

@manstis manstis left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Minor comment about consistency of the code-flow.

Comment thread src/utils/common.py
)
else:
# Service client - use sync interface
register_mcp_servers(logger, configuration)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMO it'd be better to get the client here too; to be consistent with the async approach.

async def register_mcp_servers_async(
    logger: Logger, configuration: Configuration
) -> None:
    """Register Model Context Protocol (MCP) servers with the LlamaStack client (async)."""
    if configuration.llama_stack.use_as_library_client:
        # Library client - use async interface
        # config.py validation ensures library_client_config_path is not None
        # when use_as_library_client is True
        config_path = cast(str, configuration.llama_stack.library_client_config_path)
        client = LlamaStackAsLibraryClient(config_path)
        await client.async_client.initialize()

        await _register_mcp_toolgroups_async(
            client.async_client, configuration.mcp_servers, logger
        )
    else:
        # Service client - use sync interface
        client = get_llama_stack_client(configuration.llama_stack)
    
        _register_mcp_toolgroups_sync(client, configuration.mcp_servers, logger)

Comment thread src/utils/common.py
register_mcp_servers(logger, configuration)


def register_mcp_servers(logger: Logger, configuration: Configuration) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Delete this if doing the above proposal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added it to make the tests a bit clearer (async when not using as a library).

@tisnik tisnik merged commit 4897894 into lightspeed-core:main Jun 26, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants