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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 29 Sep 2017 17:30:34 +0000 Gerrit-HasComments: Yes
