impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5041: AuthManager::Init() is not idempotent
Date Mon, 13 Mar 2017 17:05:51 GMT
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 <sailesh@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message