Todd Lipcon has posted comments on this change. Change subject: Enable GSSAPI for servers and ExternalMiniCluster ......................................................................
Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/4765/10/src/kudu/security/mini_kdc.cc File src/kudu/security/mini_kdc.cc: PS10, Line 333: kdestroy > Does it make sense to run it with '-A' option? At least, may be it makes s yep, good call on both accounts. I didn't make it optional for now. http://gerrit.cloudera.org:8080/#/c/4765/10/src/kudu/util/subprocess.cc File src/kudu/util/subprocess.cc: PS10, Line 404: // Set the environment for the subprocess. This is more portable than : // using execvpe(), which doesn't exist on OS X. : for (const auto& env : env_) { : ignore_result(setenv(env.first.c_str(), env.second.c_str(), 1 /* overwrite */)); : } > Did you also consider execve() instead of execvp()? If yes, could you add the "p" variant does shell-style PATH searching, so if we switched to 'execve', that would break. I'm not certain where/if we rely on it, but didn't want to have to go track down such cases and figure out the path, or reimplement our own $PATH searching. I'll add a small comment. -- To view, visit http://gerrit.cloudera.org:8080/4765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I595469e9cc8b2b2f57e9726014eeeb8112595801 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-HasComments: Yes
