You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2017/09/27 23:26:09 UTC
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Hello Dan Burkert, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/8157
to review the following change.
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
subprocess: Call should redirect stdout to stderr when stdout not requested
The upcoming "cluster shell" tool in the Kudu CLI will use stdout/stdin to
communicate with the user. As such, I want to make sure we're not leaking
anything into stdout, which means dealing with undesirable stdout output
generated by Subprocess::Call. That output is still useful in tests though,
so let's redirect it to stderr rather than disable it entirely.
As an alternative, I'm open to disabling stdout/stderr entirely if the
consensus is that this output just isn't interesting.
Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
---
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/subprocess.h
3 files changed, 109 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8157/1
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.cc@674
PS2, Line 674: p.RedirectStdoutToStderr();
this strikes me as odd default behavior,don't you think?
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 15:31:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h@68
PS2, Line 68: Is mutually exclusive with stream
: // disabling and sharing.
I don't understand what it means to be mutually exclusive with stream sharing. I thought the point of this was to redirect the child's stdout to stderr, and have the child's stderr shared with the parent?
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 17:19:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 3:
(1 comment)
> I don't disagree with using it for the purpose you described (ie redirect for the minicluster tool). But given this is generic utility code I'd expect it to have the same semantics as a normal fork+exec by default (i.e pass-through the parent stdout/stderr descriptors to the subprocess). If I read the patch right, it seems like the default if you just use "Subprocess::Call" is now to do the equivalent of "foo 1>&2" which is not what I'd normally expect (eg it doesn't match the python 'subprocess' module or the 'system()' function in libc)
I agree with your assessment that this policy is weird for Kudu-agnostic utility code, but how else would you suggest I "toggle" it for the minicluster tool vs. normal execution? Via gflag? It can't be via parameter because the Calls that I want to affect are, well, every Call that might be made by a tserver or master.
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h@68
PS2, Line 68: Is mutually exclusive with stream
: // disabling and sharing.
> I don't understand what it means to be mutually exclusive with stream shari
What I'm trying to say is that any particular stream may be disabled (i.e. DisableFoo), shared (default), piped (ShareParentFoo(false)), or redirected (RedirectFooToBar), but it can't be some sort of combination of those four. Each of these configurations is "mutually exclusive" with the others.
If you can think of a clearer way to convey that, I'm all ears.
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 17:30:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.cc@674
PS2, Line 674: p.RedirectStdoutToStderr();
> this strikes me as odd default behavior,don't you think?
I explained the motivation in the commit description. Could you comment on what in particular you disagree with?
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 16:26:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h@68
PS2, Line 68: Is mutually exclusive with stream
: // disabling and sharing.
> What I'm trying to say is that any particular stream may be disabled (i.e.
OK I think my confusion was that I was reading this as meaning that after calling this method, stderr couldn't be disabled or shared. Might make it clearer by substituting 'stream' with 'stdout'.
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 18:29:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 3:
> Patch Set 2:
>
> (1 comment)
I don't disagree with using it for the purpose you described (ie redirect for the minicluster tool). But given this is generic utility code I'd expect it to have the same semantics as a normal fork+exec by default (i.e pass-through the parent stdout/stderr descriptors to the subprocess). If I read the patch right, it seems like the default if you just use "Subprocess::Call" is now to do the equivalent of "foo 1>&2" which is not what I'd normally expect (eg it doesn't match the python 'subprocess' module or the 'system()' function in libc)
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 17:14:32 +0000
Gerrit-HasComments: No
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8157
to look at the new patch set (#2).
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
subprocess: Call should redirect stdout to stderr when stdout not requested
The upcoming "cluster shell" tool in the Kudu CLI will use stdout/stdin to
communicate with the user. As such, I want to make sure we're not leaking
anything into stdout, which means dealing with undesirable stdout output
generated by Subprocess::Call. That output is still useful in tests though,
so let's redirect it to stderr rather than disable it entirely.
As an alternative, I'm open to disabling stdout/stderr entirely if the
consensus is that this output just isn't interesting.
Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
---
M src/kudu/util/subprocess-test.cc
M src/kudu/util/subprocess.cc
M src/kudu/util/subprocess.h
3 files changed, 107 insertions(+), 43 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8157/2
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 2: Verified+1
Overriding Jenkins, the failure was a flaky test that I've addressed here: http://gerrit.cloudera.org:8080/8158.
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 28 Sep 2017 00:17:30 +0000
Gerrit-HasComments: No
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] subprocess: some cosmetic changes
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Alexey Serbin, Dan Burkert, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8157
to look at the new patch set (#4).
Change subject: subprocess: some cosmetic changes
......................................................................
subprocess: some cosmetic changes
Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
---
M src/kudu/util/subprocess.cc
M src/kudu/util/subprocess.h
2 files changed, 33 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8157/4
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 4
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] subprocess: some cosmetic changes
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: some cosmetic changes
......................................................................
subprocess: some cosmetic changes
Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Reviewed-on: http://gerrit.cloudera.org:8080/8157
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <da...@apache.org>
---
M src/kudu/util/subprocess.cc
M src/kudu/util/subprocess.h
2 files changed, 33 insertions(+), 19 deletions(-)
Approvals:
Kudu Jenkins: Verified
Dan Burkert: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 6
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] subprocess: some cosmetic changes
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: some cosmetic changes
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 5
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 02 Oct 2017 15:26:34 +0000
Gerrit-HasComments: No
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h@68
PS2, Line 68: Is mutually exclusive with stream
: // disabling and sharing.
> hrm. Is your minicluster tool using internal minicluster or external? if th
Todd: It's using external, though I aspire for it to use either one day. In any case, I hadn't considered handling redirection within the tool and letting it percolate down from there. I'll do that and revert this policy change.
Dan: Moot point now, because I've reverted much of this patch (it's now purely cosmetic).
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 19:53:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] subprocess: Call should redirect stdout to stderr when stdout not requested
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8157 )
Change subject: subprocess: Call should redirect stdout to stderr when stdout not requested
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h
File src/kudu/util/subprocess.h:
http://gerrit.cloudera.org:8080/#/c/8157/2/src/kudu/util/subprocess.h@68
PS2, Line 68: Is mutually exclusive with stream
: // disabling and sharing.
> What I'm trying to say is that any particular stream may be disabled (i.e.
hrm. Is your minicluster tool using internal minicluster or external? if the latter, wouldn't it suffice to just use the new Call() parameter when you start the daemons? If the former, maybe you just want to have your tool call dup2(STDERR_FILENO, STDOUT_FILENO) to redirect all stderr to stdout?
--
To view, visit http://gerrit.cloudera.org:8080/8157
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81dee93817c560c1237114fb515facbdb06b1d
Gerrit-Change-Number: 8157
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 29 Sep 2017 17:36:13 +0000
Gerrit-HasComments: Yes