Alexey Serbin has posted comments on this change. Change subject: Enable GSSAPI for servers and ExternalMiniCluster ......................................................................
Patch Set 10: (3 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 sense to have it as an options for this MiniKdc::Kdestroy() method to make sure all credentials were wiped out. PS10, Line 333: Subprocess::Call Does it make sense to check for the status code here? 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 a comment on why this setenv+execvp approach is chosen instead of execve()? -- 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 <t...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
