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.

Sign up for GitHub

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

Jump to bottom

Run command improvements #104

Closed
wants to merge 2 commits into from
Closed

Run command improvements #104

wants to merge 2 commits into from

Conversation

rkdarst
Copy link
Contributor

@rkdarst rkdarst commented Jul 31, 2018

I found this branch which I had forgotten to make a PR for.

  1. Improve error message when a run command fails (include command, eout, and status)
  2. don't risk deadlock (probably irrelevant but seems working... see discussion in Implementation of coroutines with asyncio (possibly shortens run_command substantially) #90)

rkdarst added 2 commits June 20, 2018 16:09
@rkdarst
run_command: Fix possible deadlock
d1e2408 
- Reading from stdout and stderr separately can produce a deadlock.  I
  assume that the separate proc.wait_for_exit() doesn't matter here.
- Thanks to @krinsman in jupyterhub#90.
@rkdarst
run_command: Increase debugging output in case of error
941ca27 
- Thanks to @krinsman in jupyterhub#90.
@rkdarst rkdarst mentioned this pull request Jul 31, 2018
Closed
@rkdarst
Copy link
Contributor Author

rkdarst commented Jul 31, 2018

This error is impossible to decipher, and it only happens on python 3.6.

This contained two separate things so I will close this and push them separately. The deadlock part is probably (?) causing it and not really important.

@rkdarst rkdarst closed this Jul 31, 2018
@rkdarst rkdarst deleted the run_command_improvements branch August 11, 2018 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@rkdarst