-
Notifications
You must be signed in to change notification settings - Fork 42
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
Report stacktraces for remote_exec() #102
Conversation
Besides my comment, this looks good to me. 👍 |
Thanks again @joaoe and sorry for the delay on this! 👍 |
1.7.0 released. 👍 |
Excellent. Thank you :) |
if not file_name: | ||
source = inspect.getsource(source) | ||
else: | ||
source = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line right here just broke ceph-deploy when using remote modules. At this point in the code file_name
exists along with source
, so this line sets it to None
which causes the channel to not send anything. The traceback is basically a generic EOFError
which doesn't show much, I had to step through:
> /tmp/bad/local/lib/python2.7/site-packages/execnet/gateway.py(121)remote_exec()
120 else:
--> 121 source = None
122 elif isinstance(source, types.FunctionType):
ipdb> file_name
'/home/alfredo/python/ceph-deploy/ceph_deploy/hosts/remotes.py'
ipdb> source
<module 'ceph_deploy.hosts.remotes' from '/home/alfredo/python/ceph-deploy/ceph_deploy/hosts/remotes.pyc'>
ipdb> n
> /tmp/bad/local/lib/python2.7/site-packages/execnet/gateway.py(129)remote_exec()
128
--> 129 if not call_name and kwargs:
130 raise TypeError("can't pass kwargs to non-function remote_exec")
ipdb> source
ipdb> c
I will follow up with an issue, but IMHO there was no need for any optimization here whatsoever, which puts at risk code that depends on the robustness of the (current) implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @alfredodeza
Fix #101