Avoid triggering the DeprecationWarning on os.fork #429
Conversation
|
Since we dropped Python 3.7 support, there is probably a lot of compat code that can be simplified in the resource tracker and the posix popen modules (and maybe the win32 module as well). |
|
Some of the last commits above can be reverted to use #435 instead. |
|
This should be ready for review @jeremiedbb @tomMoral @lesteve @ryandvmartin @sam-s. |
|
The EDIT: done in joblib/joblib#1683. |
| else: | ||
| passfds = sorted(passfds) |
There was a problem hiding this comment.
The lack of coverage is a false negative. This branch is actually executed when running the resource manager on Windows (to track temporary folders) but we properly do not configure coverage tracking in the resource manager process our our CI.
| is_rng = name == "n/dev/urandom" | ||
|
|
||
| # Not sure if this really expected to be open or not... | ||
| is_dev_null = name == "n/dev/null" |
There was a problem hiding this comment.
I am not 100% sure why this is needed with the new code, and it wasn't before, but keeping an open fd on /dev/null sounds harmless.
There was a problem hiding this comment.
BTW, when editing the assertion below, I checked that there was only one open fd to /dev/null and not thousands.
|
@tomMoral this is ready for review/merge. |
|
|
||
| FILE_CONTENT = """\ | ||
| import coverage; coverage.process_startup() | ||
| """ | ||
|
|
||
| filename = op.join(get_python_lib(), "coverage_subprocess.pth") | ||
| filename = op.join(get_path("platlib"), "coverage_subprocess.pth") |
There was a problem hiding this comment.
Why this change?
I did not know platlib, maybe worth adding a comment
There was a problem hiding this comment.
I stole it from @jeremiedbb's PR to add support for Python 3.13. distutils is no longer shipped in recent versions of Python. It can be reinstalled by adding setuptools as a test dependency, but it's better to stick to things available in the standard library.
There was a problem hiding this comment.
I added the comment.
Co-authored-by: Thomas Moreau <thomas.moreau.2010@gmail.com>
|
@tomMoral I addressed all your comments. Thanks for the review! |
|
Perfect, thanks! |
Fixes #424, #420.
I added some non-regression tests for each problem and checked they failed on
mainwith Python 3.13.