← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~ajorgens/cloud-init:pipe-cat into cloud-init:master

 

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.


References