core
core copied to clipboard
Run stray in threadpool to allow tool execution on http message endpoint
Description
After a debug session with @valentimarco we found that synchronous tools were not executed by the Stray if called via HTTP message endpoint, due to a problem with the event loop.
@Pingdred to the rescue, proposed to run the stray using FastAPI method run_in_threadpool.
Since the _arun method (in the procedural agent) may involve a mix of sync and async functions, using run_in_threadpool ensure that any potential blocking code is executed in a thread pool, avoiding event loop blockin.
Am I right @Pingdred, @valentimarco?
Related to issue #921
Type of change
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
Checklist:
- [X] My code follows the style guidelines of this project
- [X] I have performed a self-review of my own code
- [] I have commented my code, particularly in hard-to-understand areas
Correct!
Really delicate. Thanks for checking this @lucagobbi @valentimarco @Pingdred
In time we can add tests for sync / async tools and the /message endpoint
Hola all, after our discussion I've updated the PR to hide the "loop" complexity in the stray.run method. Now stray.run is the only method to call the stray both via HTTP and WebSocket. It accepts a flag to decide whether to return directly the result or to send it over websocket.
Personally, I don't like it very much, but as we said we hide that complexity and make the run method the only entrypoint to execute the stray.
it's time to write some tests
Thanks @lucagobbi encapsulation is good