consider-using-with on ThreadPoolExecutor?

[fluffy-critter]

Question

It's pretty common to construct a concurrent.futures.ThreadPoolExecutor and keep it around as a singleton or a global variable or the like. Until recently this passed pylint just fine, but recently it's started giving me the error:

R1732: Consider using 'with' for resource-allocating operations (consider-using-with)

Given that ThreadPoolExecutor typically is intended to sit around as a task queue that persists in the background, it seems strange for that one to be called out as a suggested use of with. Is there a particular pattern I should be following, or is this just a case where I should be suppressing the warning on a case-by-case basis?

[Pierre-Sassoulas]

Hello, I think we missed this use case. Do you think we should never raise this warning on concurrent.futures.ThreadPoolExecutor ? (Right now you can disable it, but if it's always a false positive then I think it's better if we should handle that in pylint directly). (cc @DudeNr33)

[fluffy-critter]

I assumed that the intention is to encourage people to use the with ThreadPoolExecutor() as thread_pool: at the top level of the system, but that's not always feasible, and definitely not the typical pattern for the use of c.f.TPE in my experience.

[DudeNr33]

The main problem right now is, that the check can't detect if a context manager that was constructed earlier is subsequently used in a with statement, like here:

from concurrent.futures import ThreadPoolExecutor

executor = ThreadPoolExecutor(4)

with executor:
    # do something

I will look into it as soon as possible.
Disabling the check for ThreadPoolExecutor (and then for ProcessPoolExecutor as well) would be the quick and easy solution, but maybe I can find a better way.

[DudeNr33]

I fixed the check so that the message is only triggered if the created object is not subsequently used in a with block.
@Pierre-Sassoulas should this be included in 2.9.4 or 2.10?

[fluffy-critter]

That check wouldn't actually fix the particular issue I'm facing, though. You don't generally do a bare with executor: block; either you do something like:

with ThreadPoolExecutor(4) as pool:
    pool.submit(task)

or you do:

pool = ThreadPoolExecutor(4)
pool.submit(task)

I also suspect that the close of the with will implicitly do a pool.join() which is definitely not desired in this case.

[fluffy-critter]

Yep, I just checked: the __exit__() calls join(), which is definitely not what's intended with the pattern of constructing a persistent ThreadPoolExecutor as a background worker/async task queue and submitting tasks to it.

Just because something declares __enter__/__exit__ doesn't mean that a with block is necessarily the correct usage of it. It's uncommon, but ThreadPoolExecutor is a specific example.

[Pierre-Sassoulas]

I think it should go in 2.9.4 (if possible). If we have to wait so @fluffy-critter remarks are taken into account it can go in 2.10.x.

[DudeNr33]

In this case it's probably best to remove ThreadPoolExecutor and ProcessPoolExecutor from the list of callables to check.

It seems to me like it is still best practice to call shutdown() if you use a ThreadPoolExecutor without a with statement.
However, if you call it directly you can specify wait=False if you do not want to wait until all threads are finished. The __exit__ method uses the default wait=True.
It's also a use case where I admit that it would be inconvenient to wrap your whole module/whatever inside a giant with block, and calling shutdown manually is better in terms of code style etc.

[fluffy-critter]

Yeah, it really comes down to the fact that there's different use cases for ThreadPoolExecutor and they have different patterns that make the most sense. Usually when I'm using it, it's for scheduling background tasks on a long-running persistent server process (basically in the same way one would use celery) and so there's never any reason for it to ever exit in the first place.

[DudeNr33]

Yes, you are right. Thanks for pointing that out, others would have probably run into the same issue!