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

master: plumbum.colours installs junk in sys.modules #310

Closed
jesteria opened this issue Jul 18, 2018 · 12 comments
Closed

master: plumbum.colours installs junk in sys.modules #310

jesteria opened this issue Jul 18, 2018 · 12 comments

Comments

@jesteria
Copy link
Contributor

@jesteria jesteria commented Jul 18, 2018

A script running mitogen which attempts to import pathlib raises an uncaught TypeError (exposed as CallError).

Removing the line –

import pathlib

– from my script resolves the issue.


I'm just getting started with mitogen, and really excited about it! Not only does it vastly improve our experience with Ansible; moreover, I'd like to integrate it as an extension to a high-level command (hierarchy) library, argcmdr, such that programmers at my organization can contribute to devops in whatever way suits them and their needs best (even pure Python), and such that they needn't worry per-command about remote connections or deployment.

I'm happy to see that this does work, so far, at least against the local context; except, if my command depends upon the Python 3 standard library pathlib, it fails.


Rather than include the full target code, here is an example proof, fake_manage.py:

#!/usr/bin/env python
import os
import pathlib
import socket

import mitogen.utils

def my_first_function():
    print('Hello from remote context!', socket.gethostname(), os.getpid())
    print(pathlib.Path(__file__).parent)
    return 123

def main(router):
    local = router.local()
    print(local.call(my_first_function), socket.gethostname(), os.getpid())

if __name__ == '__main__':
    mitogen.utils.log_to_file()
    mitogen.utils.run_with_router(main)

Executing the above script results in the below error output:

16:12:25.681012 E mitogen: _build_tuple('nt'): could not locate source
Traceback (most recent call last):
  File "./fake_manage.py", line 22, in <module>
    mitogen.utils.run_with_router(main)
  File "/home/user/.pyenv/versions/mtemp/lib/python3.6/site-packages/mitogen/utils.py", line 100, in run_with_router
    return func(router, *args, **kwargs)
  File "./fake_manage.py", line 17, in main
    print(local.call(my_first_function), socket.gethostname(), os.getpid())
  File "/home/user/.pyenv/versions/mtemp/lib/python3.6/site-packages/mitogen/parent.py", line 997, in call
    return receiver.get().unpickle(throw_dead=False)
  File "/home/user/.pyenv/versions/mtemp/lib/python3.6/site-packages/mitogen/core.py", line 487, in unpickle
    raise obj
mitogen.core.CallError: builtins.TypeError: must be str, not NoneType
  File "<stdin>", line 2036, in _dispatch_calls
  File "<stdin>", line 2020, in _dispatch_one
  File "<stdin>", line 367, in import_module
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 656, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 626, in _load_backward_compatible
  File "<stdin>", line 810, in load_module
  File "master:./fake_manage.py", line 3, in <module>
    import pathlib
  File "/home/user/.pyenv/versions/3.6.3/lib/python3.6/pathlib.py", line 4, in <module>
    import ntpath
  File "/home/user/.pyenv/versions/3.6.3/lib/python3.6/ntpath.py", line 523, in <module>
    from nt import _getfullpathname
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 951, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 890, in _find_spec
  File "<frozen importlib._bootstrap>", line 867, in _find_spec_legacy
  File "<frozen importlib._bootstrap>", line 441, in spec_from_loader
  File "<frozen importlib._bootstrap_external>", line 544, in spec_from_file_location
  File "<stdin>", line 817, in get_filename

As for pathlib, or really ntpath, it attempts to import nt, but anticipates an ImportError for this import on non-Windows systems. I spent some time tracing how mitogen handles (or mishandles) this case, but didn't come up with much of anything.

@dw
Copy link
Member

@dw dw commented Jul 19, 2018

Thanks for the report! The importer took quite a beating during the Python 3 work, this should hopefully be a simple fix. Will look at it closer in the coming days.

@dw
Copy link
Member

@dw dw commented Jul 19, 2018

I've pushed a fix in b5c7a440a1d9a23c9db89aec7d7652cc2e36d5ca, but it's not on master yet. Feel free to cherry-pick this if you're playing locally, otherwise give me a few days and it'll be part of the next release. :)

Thanks for reporting!

dw added a commit that referenced this issue Jul 19, 2018
On 3.x, Importer() can still have its methods called even if
load_module() raises ImportError.

Closes #310.
@dw
Copy link
Member

@dw dw commented Jul 19, 2018

Those import errors are pretty annoying -- potentially I could make them warnings instead, but that doesn't help much, since presently IoLogger (stdout/stderr of the children) are logged at the INFO level, so we can't set the default level to ERROR to hide them by default.

@jesteria
Copy link
Contributor Author

@jesteria jesteria commented Jul 19, 2018

Agreed, warning would make more sense, and at least it would be less alarming than error, regardless that it would still be displayed.

…And, wonderful! This does indeed appear to fix the issue with pathlib.

In fact, here is the result of a working draft of that integration (with argcmdr) – _build_tuple errors and all –

17:46:08.030619 E mitogen: _build_tuple('nt'): could not locate source
17:46:08.904049 E mitogen: _build_tuple('lazy_object_proxy.cext'): could not locate source
17:46:08.960580 E mitogen: _build_tuple('copy_reg'): could not locate source
17:46:09.551945 E mitogen: _build_tuple('markupsafe._speedups'): could not locate source
17:46:09.829831 E mitogen: _build_tuple('_winapi'): could not locate source
17:46:10.150205 E mitogen: _build_tuple('_yaml'): could not locate source
17:46:10.213646 I mitogen.ctx.ssh.NETLOC: stdout: hostname: HOSTNAME
17:46:10.215389 I mitogen.ctx.ssh.NETLOC: stdout: remote PID: 2433
17:46:10.216121 I mitogen.ctx.ssh.NETLOC: stdout: file path /home/USER/master:manage/server.py

– (where, of course, I'm using pathlib to generate that file path).

Unfortunately, I've got another one for you; though, it's not from the standard library, (and looking it over, I can only imagine…) – plumbum.colors.

In the script, fake_manage.py

#!/usr/bin/env python
import os
import pathlib
import socket

import mitogen.utils
from plumbum import colors

def my_first_function():
    print('hostname:', socket.gethostname())
    print('remote PID:', os.getpid())
    print('file:', pathlib.Path(__file__))
    return 123

def main(router):
    remote = router.ssh(hostname='NETLOC',
                        python_path='/usr/bin/python3.6')
    result = remote.call(my_first_function)
    print(result, socket.gethostname(), os.getpid())

if __name__ == '__main__':
    mitogen.utils.log_to_file()
    mitogen.utils.run_with_router(main)

– the line, from plumbum import colors, is enough to produce the following error output –

18:13:56.236751 E mitogen: _build_tuple('nt'): could not locate source
18:13:56.516896 E mitogen: _build_tuple('plumbum.colors'): could not locate source
Traceback (most recent call last):
  File "./fake_manage.py", line 26, in <module>
    mitogen.utils.run_with_router(main)
  File "/home/USER/dev/repo/opt/mitogen/mitogen/utils.py", line 100, in run_with_router
    return func(router, *args, **kwargs)
  File "./fake_manage.py", line 20, in main
    result = remote.call(my_first_function)
  File "/home/USER/dev/repo/opt/mitogen/mitogen/parent.py", line 1016, in call
    return receiver.get().unpickle(throw_dead=False)
  File "/home/USER/dev/repo/opt/mitogen/mitogen/core.py", line 487, in unpickle
    raise obj
mitogen.core.CallError: builtins.ImportError: Master does not have 'plumbum.colors'
  File "<stdin>", line 2049, in _dispatch_calls
  File "<stdin>", line 2033, in _dispatch_one
  File "<stdin>", line 367, in import_module
  File "<stdin>", line 811, in load_module
  File "master:./fake_manage.py", line 7, in <module>
    from plumbum import colors
  File "<stdin>", line 791, in load_module

Note, this is not reproducible with the local context – I ran across it with ssh. (And, merely importing the top-level module, plumbum, does not produce the error.)

Since plumbum isn't part of the standard library, and perhaps it's a different, (if similar), underlying issue, I'm happy to open a new, separate issue.

@dw dw closed this in d26fe5b Jul 25, 2018
@dw dw reopened this Jul 25, 2018
@dw
Copy link
Member

@dw dw commented Jul 25, 2018

Whups, didn't intend to close this, there is still some other stuff remaining

@dw
Copy link
Member

@dw dw commented Jul 29, 2018

So the problem is that plumbum installs crap in sys.modules. There used to be a third strategy to handle this but I ripped it out. Will rename this bug and look at it later. In general, there's not much to be done for modules that intentionally braindamage the interpreter, other than calling it out for what it is. :(

However, that does not mean a heuristic wouldn't have handled this case, so I still consider it something worth fixing.

@dw dw changed the title cannot import pathlib master: plumbum.colours installs junk in sys.modules Jul 29, 2018
@dw
Copy link
Member

@dw dw commented Jul 29, 2018

The relevant code is _get_module_via_parent_enumeration() as visible in master.py of e7ff625. I also need to figure out why I ripped it out before bringing it back

@dw
Copy link
Member

@dw dw commented Jul 29, 2018

I /think/ I ripped it out because I could no longer find a use for it any more -- at some stage it was needed, I think, for 'six.moves'.

@dw dw added importer and removed core labels Jul 29, 2018
@dw dw added the target:v0.2 label Aug 11, 2018
@jesteria
Copy link
Contributor Author

@jesteria jesteria commented Sep 5, 2018

Just as an update, I can confirm that the pathlib import is resolved in v0.2.2 – and that's great!

Importing plumbum.colors has the same problem, of course; however, while I agree it makes sense to keep the issue open, I should note that this is not a blocking issue, not for me, anyway. My current code paths don't actually depend on that module in the remote; and, regardless, we're talking about colors, (and which could be solved otherwise regardless).

Thanks again.

@mpdehaan
Copy link

@mpdehaan mpdehaan commented Dec 10, 2018

Currently getting something like this still, my target is a Ubuntu Bionic system with Python 3.6.6. The source is an OS X system with 3.7.0, mitogen is 0.2.3

https://gist.github.com/mpdehaan/035820f9c2322fe659178faa87ae033d

dw added a commit that referenced this issue Jan 20, 2019
- don't try anything unless something really lives in sys.modules by
  that name
- non-ASCII files are possible
- the unimportable thing might be an extension module, we don't want
  that
@dw dw closed this in 6af1a64 Jan 20, 2019
dw added a commit that referenced this issue Jan 20, 2019
* origin/dmw:
  issue #310: fix test failures, teach old import method new tricks
  master: handle crazy non-modules in sys.modules again; closes #310.
  issue #349: update Changelog.
  docs: add unused import to Changelog.
@dw
Copy link
Member

@dw dw commented Jan 20, 2019

Sorry about the huge delay on this one, all fixed now. :)

@dw
Copy link
Member

@dw dw commented Jan 20, 2019

This is now on the master branch and will make it into the next release. To be updated when a new release is made, subscribe to https://networkgenomics.com/mail/mitogen-announce/

Thanks for reporting this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants