vcstools/vcstools — pull request #8 by tkruse (2012-09-05 23:42:00+00:00)
Title: timeout for vcstools shell commands

Part of the fix to
https://github.com/vcstools/rosinstall/issues/1

Please only review the code, do not actually pull.

The code can be reviewed in general, not just the patch.

The code is tricky because:
- there is concurrency in Popen
- we often want to capture shell output
- we want to display shell output at once
- we often want to filter (supress) shell output
- there might be user interaction (entering passwords)
- some shell commands circumvent the Popen pipes

The patch is a bit dirty, because it sets a default timeout of 5 minutes, which may be unwarranted for slow connections (a rosinstall / rosws param should be introduced, for that we'd also need to extend the vcstool client API, because we do not want to extend every single function call with timeout param).

Also the patch needs to kill the running process on timeout, I think.

I chose the alarm approach for a timeout, alternative designs exist using dedicated threads, but keeping all the requirements above in order I found the code with explicit threads much worse to write and read.