use docker/cli RunExec and RunStart to handle all the interactive/tty/* terminal logic#9258
use docker/cli RunExec and RunStart to handle all the interactive/tty/* terminal logic#9258
Conversation
02cb5ac to
7d20687
Compare
rumpl
left a comment
There was a problem hiding this comment.
LGTM after cleanup of unused functions, conflict resolution and formatting :D
glours
left a comment
There was a problem hiding this comment.
LGTM, I really like this PR 🤩
Just need to fix the linter issues and remove unnecessary method before merging
89a9a70 to
a5373fd
Compare
|
There's something weird with this one: test gets an empty stdout, while running same command works. I don't get it |
…/* terminal logic Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Does it need something like https://github.com/creack/pty to have a TTY in the test? See, e.g. https://github.com/moby/moby/blob/a6919e12b103aab6eb95a67450b2a487bfec1097/integration-cli/docker_cli_attach_unix_test.go#L27-L35 |
|
ok, I eventually remembered the initial intent was to restore TTY auto-detection. Went to far into dependent changes :P |
bd0eea6 to
a2ae4fe
Compare
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
What I did
dropped all the interactive run/exec logic by leveraging RunExec/RunStart from docker/cli
Using this battle-tested code will make compose way more robust than our attempts to re-implement all corner cases
tested to confirm we don't resurect #8908