Andrew,
thanks. I think the code looks good
But a few things
a.) maybe 'detach_tty' as the name? 'no_tty=False' seems to imply that there
would be a tty, when we are not actually creating one.
b.) need some sort of test, at least to push it through here. it might be
tricky to do, but i'd like to run a subp 'detach_tty=False' and get some
subprocess to believe it is a tty.
subp(['bash', '-c', '[ "-t" 1 ] && echo attached to tty || echo not attached
to tty'])
the difficulty is that in any c-i or automated environment, we're probably
not attached to a tty. but we do need some test of this code path.
c.) i think best to take the detach_tty path only if we *have* a tty.
if os.isatty(sys.stdout):
... go the detach route
else:
... no need to detach
--
https://code.launchpad.net/~ajorgens/cloud-init/+git/cloud-init/+merge/325512
Your team cloud-init commiters is requested to review the proposed merge of
~ajorgens/cloud-init:pipe-cat into cloud-init:master.
_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
