msg321324 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-07-09 15:29 |
I've had a few conversations with people who were confused that asyncio starts to behave weirdly when a ProcessPoolExecutor is set as the default one. We don't really test that asyncio's built-in functionality (like DNS resolving) works well with a process-pool, which leads to bug reports like [1]. Third-party libraries also always assume that the loop is always configured to use the ThreadPoolExecutor (as it is by default), and also don't even test against ProcessPool. My idea here would be to deprecate setting ProcessPoolExecutor as a default one in 3.8 and prohibit that in 3.9. Guido, Andrew, what do you think? [1] https://bugs.python.org/issue34073 |
|
|
msg321330 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2018-07-09 16:12 |
Yeah, that's a good idea. It was never meant for a ProcessPoolExecutor. We should warn against this in the docs right away (and backport the warning to all previous versions that have set_executor). I also support non-silent deprecation in 3.8. |
|
|
msg321331 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-07-09 16:16 |
Great! Thanks for the quick reply, Guido. |
|
|
msg321338 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-07-09 18:23 |
> We should warn against this in the docs right away (and backport the warning to all previous versions that have set_executor). I think we'll only allow instances of c.f.ThreadPoolExecutor (and its subclasses) to be passed to set_default_executor. That's a more robust check than just guarding against ProcessPoolExecutor. |
|
|
msg321339 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2018-07-09 18:24 |
Of course, that's what I meant. |
|
|
msg321459 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-07-11 14:34 |
> I think we'll only allow instances of c.f.ThreadPoolExecutor (and its subclasses) to be passed to set_default_executor. That's a more robust check than just guarding against ProcessPoolExecutor. I suggest to only reject ProcessPoolExecutor, instead of having a very specific test on ThreadPoolExecutor which might fail with funky but valid thread executor. |
|
|
msg321469 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2018-07-11 14:53 |
I disagree. Other than subclasses of thread executor, what are you going to pass in? A mock? On Wed, Jul 11, 2018 at 7:34 AM STINNER Victor <report@bugs.python.org> wrote: > > STINNER Victor <vstinner@redhat.com> added the comment: > > > I think we'll only allow instances of c.f.ThreadPoolExecutor (and its > subclasses) to be passed to set_default_executor. That's a more robust > check than just guarding against ProcessPoolExecutor. > > I suggest to only reject ProcessPoolExecutor, instead of having a very > specific test on ThreadPoolExecutor which might fail with funky but valid > thread executor. > > ---------- > nosy: +vstinner > > _______________________________________ > Python tracker <report@bugs.python.org> > <https://bugs.python.org/issue34075> > _______________________________________ > -- --Guido (mobile) |
|
|
msg321476 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-07-11 15:08 |
> Other than subclasses of thread executor, what are you going to pass in? I don't see why asyncio should prevent people to experiment their own custom executor. You can imagine a custom "green executor" which inherit from Executor but uses its own black magic like greenlet. People like to do crazy things to implement asynchronous programming :-) |
|
|
msg321479 - (view) |
Author: Yury Selivanov (yselivanov) *  |
Date: 2018-07-11 15:14 |
> I don't see why asyncio should prevent people to experiment their own custom executor. You can imagine a custom "green executor" which inherit from Executor but uses its own black magic like greenlet. Because asyncio and its ecosystem is built around the fact that the default executor is a ThreadPoolExecutor. asyncio users are free to: 1. Pass any kind of executor as a first argument to `loop.run_in_executor()` -- no restrictions there. 2. Pass a subclass of ThreadPoolExecutor to `loop.set_default_executor()` -- that's how people can experiment. Mind that (2) also covers exotic use cases like using greenlets under the hood (although I don't think it's possible), as long as they subclass ThreadPoolExecutor. My plan so far: * in 3.8 add a DeprecationWarning if an executor other than ThreadPoolExecutor is set as a default one. * during 3.8 see what kind of feedback we get. * if all goes great, in 3.9 we'll only allow subclasses of ThreadPoolExecutor. |
|
|
msg321620 - (view) |
Author: Andrew Svetlov (asvetlov) *  |
Date: 2018-07-13 15:29 |
Agree, restricting to ThreadPoolExecutor sounds reasonable. A custom executor may have the same problem as ProcessPoolExecutor. Moreover it can work under same scenarios but crash with other third-party libs. |
|
|
msg322665 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-07-30 10:42 |
New changeset 22d25085db2590932b3664ca32ab82c08f2eb2db by Victor Stinner (Elvis Pranskevichus) in branch 'master': bpo-34075: Deprecate non-ThreadPoolExecutor in loop.set_default_executor() (GH-8533) https://github.com/python/cpython/commit/22d25085db2590932b3664ca32ab82c08f2eb2db |
|
|
msg322666 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2018-07-30 10:43 |
Elvis Pranskevichus: well done, your first PR was already good to be merged ;-) |
|
|
msg338872 - (view) |
Author: Bas Nijholt (basnijholt) |
Date: 2019-03-26 11:40 |
I think this issue is related to the problem in https://bugs.python.org/issue36281 If it indeed is the case, then the fix proposed here and implemented in https://github.com/python/cpython/commit/22d25085db2590932b3664ca32ab82c08f2eb2db won't really help. |
|
|