Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Options with nonserializable types break under xdist #384

Open
Hawk777 opened this issue Dec 1, 2018 · 10 comments
Open

Options with nonserializable types break under xdist #384

Hawk777 opened this issue Dec 1, 2018 · 10 comments
Labels

Comments

@Hawk777
Copy link

Hawk777 commented Dec 1, 2018

I wanted a command-line option to take a filename as its value. It works fine with vanilla py.test, but not with xdist.

$ cat test_stuff.py 
def test_stuff():
    pass

$ cat conftest.py 
import pathlib
import pytest
@pytest.hookimpl
def pytest_addoption(parser):
    parser.addoption("--my-option", type=pathlib.Path)

$ py.test --my-option=/
============================= test session starts ==============================
platform linux -- Python 3.6.5, pytest-4.0.1, py-1.7.0, pluggy-0.8.0
rootdir: /tmp/test, inifile:
plugins: xdist-1.24.1, forked-0.2
collected 1 item                                                               

test_stuff.py .                                                          [100%]

=========================== 1 passed in 0.01 seconds ===========================

$ py.test --my-option=/ -n2
================================================== test session starts ==================================================
platform linux -- Python 3.6.5, pytest-4.0.1, py-1.7.0, pluggy-0.8.0
rootdir: /tmp/test, inifile:
plugins: xdist-1.24.1, forked-0.2
gw0 C / gw1 IINTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1383, in _save
INTERNALERROR>     dispatch = self._dispatch[tp]
INTERNALERROR> KeyError: <class 'pathlib.PosixPath'>
INTERNALERROR> 
INTERNALERROR> During handling of the above exception, another exception occurred:
INTERNALERROR> 
INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/_pytest/main.py", line 183, in wrap_session
INTERNALERROR>     config.hook.pytest_sessionstart(session=session)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/hooks.py", line 284, in __call__
INTERNALERROR>     return self._hookexec(self, self.get_hookimpls(), kwargs)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/manager.py", line 67, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook, methods, kwargs)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/manager.py", line 61, in <lambda>
INTERNALERROR>     firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/callers.py", line 208, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/callers.py", line 80, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/pluggy/callers.py", line 187, in _multicall
INTERNALERROR>     res = hook_impl.function(*args)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/dsession.py", line 81, in pytest_sessionstart
INTERNALERROR>     nodes = self.nodemanager.setup_nodes(putevent=self.queue.put)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/workermanage.py", line 67, in setup_nodes
INTERNALERROR>     nodes.append(self.setup_node(spec, putevent))
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/workermanage.py", line 76, in setup_node
INTERNALERROR>     node.setup()
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/xdist/workermanage.py", line 246, in setup
INTERNALERROR>     self.channel.send((self.workerinput, args, option_dict))
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 717, in send
INTERNALERROR>     self.gateway._send(Message.CHANNEL_DATA, self.id, dumps_internal(item))
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1354, in dumps_internal
INTERNALERROR>     return _Serializer().save(obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1372, in save
INTERNALERROR>     self._save(obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1390, in _save
INTERNALERROR>     dispatch(self, obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1475, in save_tuple
INTERNALERROR>     self._save(item)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1390, in _save
INTERNALERROR>     dispatch(self, obj)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1471, in save_dict
INTERNALERROR>     self._write_setitem(key, value)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1465, in _write_setitem
INTERNALERROR>     self._save(value)
INTERNALERROR>   File "/tmp/venv/lib64/python3.6/site-packages/execnet/gateway_base.py", line 1388, in _save
INTERNALERROR>     raise DumpError("can't serialize %s" % (tp,))
INTERNALERROR> execnet.gateway_base.DumpError: can't serialize <class 'pathlib.PosixPath'>
@nicoddemus
Copy link
Member

Indeed, that's a limitation of our backend, we should add this to the documentation.

In addition, we could also register a serializer for a Path object.

@Hawk777
Copy link
Author

Hawk777 commented Dec 1, 2018

I had assumed that the options were passed textually and reparsed in the child. Obviously that would not have had this limitation, since the CLI parameters were strings to start with before argparse got its hands on them.

@nicoddemus
Copy link
Member

You are right, but to reparse in the worker, you need specialized code for it that knows about pathlib.Path objects, and currently execnet does not do it, which is unfortunate. 😕

@Hawk777
Copy link
Author

Hawk777 commented Dec 1, 2018

Oh, no, I meant I assumed that the options weren’t passed as parsed objects at all. I originally assumed that sys.argv was handed over from parent to child, and of course sys.argv has nothing but strings in it, so then all the options could just be reparsed in the child. So nobody would need to know about Path objects, because they’d just be strings during marshalling.

@nicoddemus
Copy link
Member

nicoddemus commented Dec 1, 2018

Oh I see what you mean, makes sense. It seems self.channel.send((self.workerinput, args, option_dict)) is trying to send the parsed options directly instead of sys.argv, which explains the error.

I believe (without looking at the code) that xdist will first parse and handle/filter out some of the arguments, because workers won't know what to do with some options (-n for example). Not sure how easy is this to fix, it would need more investigation I'm afraid.

@nicoddemus nicoddemus added the bug label Dec 1, 2018
@Hawk777
Copy link
Author

Hawk777 commented Dec 1, 2018

Yes, I realize it will need some investigation. I guess there might be cases where, if workers are being run on other machines, then xdist might not be installed on the worker machines so they might not understand -n. Or even if not that, I suppose the provision of -n might make the worker try to spawn more workers… Could be complicated, but I think it might be helpful to do sometime.

@RonnyPfannschmidt
Copy link
Member

structurally pytest itself simply does not support required arguments at all
we should really warn or error on it instead of failing at "interesting" places

@Hawk777
Copy link
Author

Hawk777 commented Dec 1, 2018

@RonnyPfannschmidt comment on wrong ticket?

@RonnyPfannschmidt
Copy link
Member

@Hawk777 whops, my fault, one shouldn't open all the images right after a oversize dinner

@graingert
Copy link
Member

@nicoddemus it looks like this is still happening and options_dict is still being sent

@graingert graingert reopened this Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants