core icon indicating copy to clipboard operation
core copied to clipboard

Run stray in threadpool to allow tool execution on http message endpoint

Open lucagobbi opened this issue 1 year ago • 2 comments

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

lucagobbi avatar Oct 03 '24 06:10 lucagobbi

Correct!

valentimarco avatar Oct 03 '24 07:10 valentimarco

Really delicate. Thanks for checking this @lucagobbi @valentimarco @Pingdred

In time we can add tests for sync / async tools and the /message endpoint

pieroit avatar Oct 03 '24 10:10 pieroit

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.

lucagobbi avatar Oct 05 '24 16:10 lucagobbi

it's time to write some tests

valentimarco avatar Oct 05 '24 16:10 valentimarco

Thanks @lucagobbi encapsulation is good

pieroit avatar Oct 06 '24 14:10 pieroit