batchspawner/batchspawner: Fix exception handling in run_command.

jupyterhub / batchspawner

Custom Spawner for Jupyterhub to start servers in batch scheduled systems

BSD 3-Clause "New" or "Revised" License

190 stars 134 forks source link

batchspawner/batchspawner: Fix exception handling in run_command. #195

Closed olifre closed 3 years ago

olifre commented 3 years ago

The exception handling in run_command had several issues which this commit fixes:

Don't use undefined variable.
communicate() may block forever if process already closed the pipes.
exceptions during exception handling were not handled.

welcome[bot] commented 3 years ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. welcome You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

consideRatio commented 3 years ago

This LGTM and is a obvious bugfix! Thank you @olifre :heart: :tada:

Thank you for your clear PR summary, it provided very relevant help for me to review this!

olifre commented 3 years ago

@consideRatio Many thanks for merging and for the positive feedback, this really makes contributing a delight :smile: .