Henry Robinson has posted comments on this change. Change subject: IMPALA-5041: AuthManager::Init() is not idempotent ......................................................................
Patch Set 1: (5 comments) Thanks for doing this. http://gerrit.cloudera.org:8080/#/c/6333/1//COMMIT_MSG Commit Message: PS1, Line 7: idempotent nit: do you really mean idempotent? That means calling it twice gets the same results. I think you mean that it calling it twice (maybe with different params?) is not possible. PS1, Line 15: enviornment environment http://gerrit.cloudera.org:8080/#/c/6333/1/be/src/rpc/authentication.cc File be/src/rpc/authentication.cc: PS1, Line 1033: CheckReplayCacheDirPermissions move this inside InitKerberosEnv()? Seems to belong there. http://gerrit.cloudera.org:8080/#/c/6333/1/be/src/rpc/authentication.h File be/src/rpc/authentication.h: PS1, Line 39: ystem-wide authentication manager responsible for initialising authentication systems, : /// including SSL, Sasl and Kerberos, and for providing auth-enabled Thrift structures to : /// servers and clients. Can you expand this to talk about how the auth manager should be used? Can there be more than one simultaneously (no?), can there be two consecutively? Since we're removing a code-based check, we should at least document the usage expectations. PS1, Line 63: InitKerberosEnv private? -- To view, visit http://gerrit.cloudera.org:8080/6333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12cc210aa422f077446ea728ebf76921351417b0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-HasComments: Yes
