search

lordmauve / chopsticks

Chopsticks is an orchestration library: it lets you execute Python code on remote hosts over SSH.
https://chopsticks.readthedocs.io/
Apache License 2.0
158 stars 16 forks source link

Context Manager and timeout on `self.wpipe.close` #16

Closed romanlevin closed 7 years ago

romanlevin commented 7 years ago

This is kind of rough at the moment (at very least, the timeout needs to be configurable), but I was wondering if you'd be interested in a cleaned up version of this.

romanlevin commented 7 years ago

Pinging @lordmauve

lordmauve commented 7 years ago

Hi, sorry for the delay.

I think it might be a better idea for close() to simply signal the process in the default case, rather than waiting.

I wouldn't be averse to having the stronger version - wait for process to die, or kill it - with something like close(force=True).

romanlevin commented 7 years ago

@lordmauve I replaced timeout with a polling loop. Do you still think it should just not wait by default?

romanlevin commented 7 years ago

@lordmauve I've tested with both Python 2.7.13 (wheren Popen.stdin is a file descriptor) and with Python 3.6.1 (where it is an io.BufferedWriter), and both allow multiple calls to close().

Any subsequent calls are noops.

lordmauve commented 7 years ago

Ok, then!