Adar Dembo has posted comments on this change. Change subject: subprocess: add ScopedSubprocess ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/util/subprocess.h File src/kudu/util/subprocess.h: Line 185: ~ScopedSubprocess() { > nit: given this is a pretty heavy-weight function, i think it's better to d Done Line 190: WARN_NOT_OK(p_.Kill(signum_), "Could not kill subprocess"); > would be useful to include argv[0] or something in these warning messages h Done Line 191: Status s = p_.Wait(); > should there be any kind of timeout and fallback to kill -9? Done PS1, Line 193: WARNING > error here and below Done -- To view, visit http://gerrit.cloudera.org:8080/6741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
