Skip to content

Conversation

pgrayy
Copy link
Member

@pgrayy pgrayy commented Oct 9, 2025

This reverts commit 08dc4ae.

Description

When the session manager is executed in an async environment, the asyncio.run will fail with a "cannot be called from a running event loop" error. This was introduced in #897 as a means to improve the performance of session reads. As a fix, we could wrap the asyncio.run in a thread similar to what we do elsewhere in code. However, since this is a breaking change, we are going to revert the change and work on the fix separately.

Note, I made these changes with git revert.

Related Issues

#1012

Documentation PR

N/A

Type of Change

Bug fix

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

  • I ran hatch run prepare: We can rely on the existing tests as this is a revert of an optimization rather than a behavioral change.

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/strands/session/s3_session_manager.py 83.33% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@dbschmigelski
Copy link
Member

dbschmigelski commented Oct 9, 2025

I'm adding

def run_async(async_func: Callable[[], Awaitable[T]]) -> T:
    """Run an async function in a separate thread to avoid event loop conflicts.

    This utility handles the common pattern of running async code from sync contexts
    by using ThreadPoolExecutor to isolate the async execution.

    Args:
        async_func: A callable that returns an awaitable

    Returns:
        The result of the async function
    """

    async def execute_async() -> T:
        return await async_func()

    def execute() -> T:
        return asyncio.run(execute_async())

    with ThreadPoolExecutor() as executor:
        future = executor.submit(execute)
        return future.result()

in #895 which can be used here. But this is still unfortunate, because we are penalizing people who are async from the start to make it easier for the sync builders.

Maybe we can abstract things further by running in the existing event loop if one already exists

@pgrayy
Copy link
Member Author

pgrayy commented Oct 9, 2025

I'm adding

def run_async(async_func: Callable[[], Awaitable[T]]) -> T:
    """Run an async function in a separate thread to avoid event loop conflicts.

    This utility handles the common pattern of running async code from sync contexts
    by using ThreadPoolExecutor to isolate the async execution.

    Args:
        async_func: A callable that returns an awaitable

    Returns:
        The result of the async function
    """

    async def execute_async() -> T:
        return await async_func()

    def execute() -> T:
        return asyncio.run(execute_async())

    with ThreadPoolExecutor() as executor:
        future = executor.submit(execute)
        return future.result()

in #895 which can be used here. But this is still unfortunate, because we are penalizing people who are async from the start to make it easier for the sync builders.

Maybe we can abstract things further by running in the existing event loop if one already exists

Yes and we can check for that and should. asyncio has some utility methods to grab the current running event loop if it exists.

@pgrayy pgrayy merged commit 92da544 into strands-agents:main Oct 9, 2025
14 of 27 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