impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4965: Authorize access to runtime profile and exec summary
Date Wed, 07 Jun 2017 23:49:42 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4965: Authorize access to runtime profile and exec summary
......................................................................


Patch Set 7:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7064/7/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

Line 218:       const ClientRequestState& request_state);
why can't this be a regular method on 'this'?
also, function comment.


Line 327:   bool user_has_profile_access_;
comment what exactly this means (like you have in QueryStateRecord)


http://gerrit.cloudera.org:8080/#/c/7064/7/be/src/service/impala-beeswax-server.cc
File be/src/service/impala-beeswax-server.cc:

PS7, Line 355: GetSessionState(session_id, &session)
you should just pass '&session' as the second param to WithSession to get this.


PS7, Line 375: GetSessionState(session_id, &session)
you should just pass '&session' as the second param to WithSession to get this.


http://gerrit.cloudera.org:8080/#/c/7064/7/be/src/service/impala-server.h
File be/src/service/impala-server.h:

Line 638:       const QueryStateRecord& query_record);
same, why not regular method?
also add function comment.


http://gerrit.cloudera.org:8080/#/c/7064/7/be/src/util/auth-util.h
File be/src/util/auth-util.h:

Line 45: Status CheckProfileAccess(const std::string& user, const std::string& effective_user,
this needs documentation, including what the parameters mean. i.e. not obvious from decl what
'user' really is. and really, aren't they both "effective users"? one associated with the
current session and one with the original query?


-- 
To view, visit http://gerrit.cloudera.org:8080/7064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2255d587367c2d328590ae8534a5406c4b0c9b15
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message