-
Notifications
You must be signed in to change notification settings - Fork 949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bugfix: disable mux stdout and stderr if backend is not http #1250
Conversation
Signed-off-by: YaoZengzeng <[email protected]>
Stdout: true, | ||
Stderr: true, | ||
MemBuffer: &output, | ||
MuxDisabled: true, | ||
} | ||
|
||
err = c.ContainerMgr.StartExec(ctx, execid, startConfig, attachConfig) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use pipeIO to check the status? I think that the spin check is not good for the performance.
BTW, if the cmd is top, the timeout can work with the io.Copy
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's ugly actually, I'll make it more elegant in the future.
Thanks for your advice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add an issue to record this? @fuweid
Or @YaoZengzeng you to add this issue?
And thanks for the test fixing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@allencloud @YaoZengzeng I will add the issue for this. Thanks
Codecov Report
@@ Coverage Diff @@
## master #1250 +/- ##
==========================================
+ Coverage 15.27% 15.28% +0.01%
==========================================
Files 172 172
Lines 10691 10682 -9
==========================================
Hits 1633 1633
+ Misses 8938 8929 -9
Partials 120 120
|
LGTM |
Signed-off-by: YaoZengzeng [email protected]
Ⅰ. Describe what this PR did
Yesterday we skip some tests of CRI.
It's so dangerous, So I make a quick fix to make it all right :)
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how you did it
Ⅳ. Describe how to verify it
Ⅴ. Special notes for reviews