Skip to content
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

Method to run code in a thread #85

Closed
willmcgugan opened this issue Sep 4, 2021 · 16 comments
Closed

Method to run code in a thread #85

willmcgugan opened this issue Sep 4, 2021 · 16 comments
Labels
help wanted Extra attention is needed level:moderate A task require a bit of thought

Comments

@willmcgugan
Copy link
Collaborator

I'd like to add a convenient method to execute in job in a thread.

It would probably be added to message pump (used by app / widgets).

It should allow a method to be called with arbitrary parameters, and should return an awaitable.

await self.call_threaded(my_code, "foo", "bar")

I can see this being used for jobs such as reading from disk (which at the moment blocks the vent loop).

Some considerations: a thread pool may be appropriate. We don't want this to be unbounded and potentially launch an unlimited number of threads. It should probably work with both async and sync functions, which will require a little inspection. If you want to tackle this, please discuss your ideas here first.

@willmcgugan willmcgugan added help wanted Extra attention is needed level:moderate A task require a bit of thought labels Sep 4, 2021
@ksamuel
Copy link

ksamuel commented Sep 4, 2021

Asyncio already provides facilities for that:

https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor

However, we could provide a wrapper that will:

  • get_event_loop automatically
  • use the default executor instead of requesting None as a first parameter
  • wrap the callable and any excess param into functiools.partial prior to passing it to the thread
  • if the callable returns a coroutine or if passed an awaitable, we call create_task from a different event loop within the wrapper and pass it to run_until_complete

What's message pump ?

Edit : got it, message pump seems to be a queue based pub/sub infra to propagate event in the app.

@ksamuel
Copy link

ksamuel commented Sep 4, 2021

So what would you want to add in message pump ?

@willmcgugan
Copy link
Collaborator Author

A call_threaded method, so it is inherited by all the components.

@ksamuel
Copy link

ksamuel commented Sep 4, 2021

Is there any reason to add it to the message pump other than making it accessible to all components ?

@willmcgugan
Copy link
Collaborator Author

It makes it easy to log the component making the call. I suspect there are also other benefits in having a strong association with the component that initiated the code, but they escape me ATM.

@adriangb
Copy link

adriangb commented Sep 4, 2021

Something like this (originally adapted from Starlette): https://github.com/adriangb/anydep/blob/b18d45ade01d8993d348e72284dde43b8eb612c2/anydep/concurrency.py#L23 ?

@willmcgugan
Copy link
Collaborator Author

Quite similar, yes.

@adriangb
Copy link

adriangb commented Sep 4, 2021

I think the main advantage of that method over the originally proposed one or Starlette's (here) is that it does not immediately bind *args, **kwargs, so you get more flexibility. You can "convert" the blocking callable to an async callable in one place, and call it in another:

if not (# is async callable):
    my_code = callable_in_thread_pool(my_code)
# do some other stuff
await  my_code("foo", "bar")

Or call it in the same place by:

await  callable_in_thread_pool(my_code)("foo", "bar")

@adriangb
Copy link

adriangb commented Sep 4, 2021

You also said that:

It should probably work with both async and sync functions

I think for this it makes sense to have a separate method. Maybe something like:

@overload
def guarantee_coroutine(callable: Callable[..., Awaitable[T]]) -> Callable[..., Awaitable[T]]:
    ...

@overload
def guarantee_coroutine(callable: Callable[..., T]) -> Callable[..., Awaitable[T]]:
    ...

def guarantee_coroutine(callable: Any) -> Callable[..., Awaitable[T]]:
    if not inspect.isroutine(call):
        return callable_in_thread_pool(call)
    return call

@lllama
Copy link
Contributor

lllama commented Sep 5, 2021

It might be worth seeing how unsync does this. (https://github.com/alex-sherman/unsync). I believe it maintains some sort of thread pool and let's you mix sync and async. (Plus it's only about 200 lines of code.)

@Tinche
Copy link

Tinche commented Sep 24, 2021

Does Textual run in an actual asyncio event loop, or does it use something custom? If it's just asyncio under the hood, there are libraries like aiofiles that'll give you async files through a thread pool.

@adriangb
Copy link

@Tinche it just runs an asyncio loop: https://github.com/willmcgugan/textual/blob/f574294beb733df0cfd17993401713edf5ed7fca/src/textual/app.py#L188

Using a library that already an async interface to sync tasks is always an option. I think this is for the case when that is not immediately an option.

@Tinche
Copy link

Tinche commented Sep 30, 2021

@adriangb I see, that's pretty cool. Tangentially: if it was up to me I'd actually advertise the asyncio.run approach of running Textual in the docs instead of App.run, since it makes it clearer how Textual works conceptually and that, like in any asyncio process, you're free to run other coroutines in parallel with Textual.

That said, as mentioned upthread asyncio already has a way to run things on a thread pool.

@tusharsadhwani
Copy link
Contributor

I think App.run is important to keep things simple by default, some people don't know anything about asyncio.

I'd suggest showing both approaches, asyncio one probably in an advanced section somewhere.

@willmcgugan
Copy link
Collaborator Author

@github-actions
Copy link

Did we solve your problem?

Consider buying the Textualize developers a coffee to say thanks.

Textualize

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed level:moderate A task require a bit of thought
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants