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

subprocess call should check return status #18

Closed
caodg opened this issue Apr 5, 2016 · 3 comments
Closed

subprocess call should check return status #18

caodg opened this issue Apr 5, 2016 · 3 comments
Assignees
Labels
Milestone

Comments

@caodg
Copy link
Member

caodg commented Apr 5, 2016

not just calling sys_run and assume it will execute correctly. sometimes it will fail and the failure info will get lost.

@caodg caodg added the bug label Apr 5, 2016
@caodg caodg added this to the v0.2.7 milestone Apr 5, 2016
@zhongyehong
Copy link
Member

I have checked returncode after calling sys_run, the bug today is i call (lxc-stop -k -n %s) but not ((lxc-stop -k -n %s) % lxc_name). So the shell regard "%s" as lxc_name. I have checked the returncode after calling this, but the calling is a "grep RUNNING". It return not 0, the same with a stopped container. Docklet misakes it for a stopped container. It is not because that I call sys_run without checking returncode.

@caodg
Copy link
Member Author

caodg commented Apr 6, 2016

you mean checking like this?

 if status == "running":
        sys_run("lxc-stop -k -n %s" % lxc_name)
[success, status] = self.container_status(lxc_name)

should be

Ret = sys_run(...)
if Ret.returncode == 0:

I recommend not encapsulating sys call in sys_run, you can directly call subprocess.run with check=True, or we do not need to using this python 3.5 new api.

zhongyehong added a commit to zhongyehong/docklet that referenced this issue Apr 11, 2016
use sys_run instead of self.sys_call in imagemgr.py
zhongyehong added a commit that referenced this issue Apr 11, 2016
mount tmp from host #39 move sys_run code to try block #18
@zhongyehong
Copy link
Member

Done

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

2 participants