impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <>
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:


Thanks for doing this.
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
File be/src/rpc/

PS1, Line 1033: CheckReplayCacheDirPermissions
move this inside InitKerberosEnv()? Seems to belong there.
File be/src/rpc/authentication.h:

PS1, Line 39: ystem-wide authentication manager responsible for initialising authentication
            : /// 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

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I12cc210aa422f077446ea728ebf76921351417b0
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-HasComments: Yes

View raw message