Author aeros
Recipients aeros, asvetlov, primal, yselivanov
Date 2019-11-02.00:57:57
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1572656277.92.0.719838239937.issue32309@roundup.psfhosted.org>
In-reply-to
Content
> async with asyncio.ThreadPool(concurrency=10) as pool:

I'm definitely on board with the usage of an async context manager and the functionality shown in the example, but I'm not sure that I entirely understand what the "concurrency" kwarg in "concurrency=10" is supposed to represent in this case. Could you elaborate on what that would do functionally?

> It should be the "async with".

> We should not reuse the default executor.

> I think it's OK to initialize the thread pool in `ThreadPool.__init__`.  `ThreadPool.__aenter__` would simply return `self` then.

> I think we should aim for shipping a replacement for `loop.run_in_executor` in 3.9.

Strong +1 on all of those points, I agree. 

> `await ThreadPool.aclose()` would close the loop gracefully (awaiting for all submitted and still running callables) in cases when people use the threadpool without 'async with'.

I believe behavior occurs within shutdown_default_executor(), correct? Specifically, within for ThreadPoolExecutor when executor.shutdown(wait=True) is called and all of the threads are joined without a timeout, it simply waits for each thread to terminate gracefully. 

So if we moved that functionality to a private coroutine method _shutdown_executor() as suggested in my above example, this could also be done for ThreadPool. Unless you want ThreadPool to be it's own entirely separate implementation that doesn't depend at all on ThreadPoolExecutor. 

Personally I think we can use ThreadPoolExecutor in the internal details; it seems that the main issue with it isn't the functionality, but rather the low level API offered with loop.run_in_executor().

Also another point to consider is if we should have users explicitly call pool.aclose() or if this should be done automatically when exiting the context manager through the __aexit__. I prefer the latter myself for similar reasons to what I previously mentioned, with a context manager initializing it's own resources on entry and finalizing them upon exit.
History
Date User Action Args
2019-11-02 00:57:57aerossetrecipients: + aeros, asvetlov, yselivanov, primal
2019-11-02 00:57:57aerossetmessageid: <1572656277.92.0.719838239937.issue32309@roundup.psfhosted.org>
2019-11-02 00:57:57aeroslinkissue32309 messages
2019-11-02 00:57:57aeroscreate