[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public

Conversation

Copy link
Contributor

mcepl commented Mar 19, 2020

Skip test_get_event_loop_new_process on systems which don’t provide it
(e.g., building environments of some Linux distributions).

https://bugs.python.org/issue38377

Automerge-Triggered-By: @pitrou

Copy link
Member

cc @pablogsal @pitrou @applio

Copy link
Member

pitrou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 from me

mcepl added 2 commits June 16, 2020 21:46
Skip test_get_event_loop_new_process on systems which don’t provide it
(e.g., building environments of some Linux distributions).
Instead of doing things automagically, add new environmental
variable PYTHON_NO_DEV_SHM, which switches off particular tests.
Copy link
Contributor

@mcepl: Status check is done, and it's a failure ❌ .

Copy link
Member

pitrou commented Jun 16, 2020

@mcepl You should have pulled from the updated PR instead of force-pushing :-(

Copy link
Member

pitrou commented Jun 16, 2020

Please reapply the lost changes now.

Copy link
Contributor Author

mcepl commented Jun 17, 2020

Please reapply the lost changes now.

What lost changes? I have rebased my branch on the top of the current master and add one more commit. What did I loose? The change is still the same.

I am sorry, what I am missing here?

Copy link
Member

pitrou commented Jun 17, 2020

Yes, you rebased and force-pushed while I had pushed another commit, so you lost it.

Copy link
Member

pitrou commented Jun 17, 2020

See 8fd3d0c

Copy link
Member

tiran commented Jun 17, 2020

Did you consider to implement auto-detection of SHM interface instead of an env var? Something like

def has_shm():
    if sys.platform == 'linux':
   	     with open("/proc/mounts") as f:
             return "/dev/shm" in f.read()
     else:
         ...


if sys.platform != 'win32':

@unittest.skipIf(os.environ.get('PYTHON_NO_DEV_SHM') == '1',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit-pick, you should also check sys.flags.ignore_environment here.

Copy link
Contributor Author

mcepl commented Jun 17, 2020

Did you consider to implement auto-detection of SHM interface instead of an env var? Something like

@tiran Isn’t this exact opposite of what Victor asked for in #19073 (comment) ?

Copy link
Member

tiran commented Jun 17, 2020

Did you consider to implement auto-detection of SHM interface instead of an env var? Something like

@tiran Isn’t this exact opposite of what Victor asked for in #19073 (comment) ?

Ah, I didn't see the comments because they were not on BPO and GH hides coments of closed discussions.

Copy link
Member

@mcepl:

@tiran Isn’t this exact opposite of what Victor asked for in #19073 (comment) ?

My first remark was:

I'm not sure if /dev/shm should be hardcoded here.

@tiran:

Did you consider to implement auto-detection of SHM interface instead of an env var?

That would avoid hardcoding and so it sounds like a reasonable approach.

I also wrote:

I dislike that test_asyncio becomes tidely coupled to multiprocessing internals. Is there a function in multiprocessing somewhere to check if /dev/shm is required and usable?

If code is written, I suggest to add a private function to multiprocessing.util. See for example _cleanup_tests() there which is used by multiprocessing tests but also by concurrent.futures tests.


To come back to https://bugs.python.org/issue38377 it seems like the problem is that creating _multiprocessing.SemLock() raises an OSError. There are other tests which are skipped if SemLock doesn't work. Example with test_concurrent_futures:

# Skip tests if sem_open implementation is broken.
support.import_module('multiprocessing.synchronize')

Maybe multiprocessing.synchronize should raises an exception on import on Linux if /dev/shm, rather than trying to put logic far from the code?

The simplest check is to create a SemLock() object and destroy it. But maybe a different approach which doesn't avoid creating a lock can be found?

Another simple check is to raise an ImportError on Linux if /dev/shm/ directory doesn't exist.

/dev/shm path is already hardcoded in Lib/multiprocessing/heap.py: Arena._dir_candidates.

Copy link
Member

I wrote a different approach: PR #20944 attempts to create a lock and convert OSError to unittest.SkipTest exception.

Copy link
Contributor Author

mcepl commented Jun 18, 2020

Closing this in preference to #20944.

mcepl closed this Jun 18, 2020
mcepl deleted the 38377_multiprocessing-requires-dev_shm branch June 18, 2020 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting changes tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants