impala-reviews mailing list archives

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

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


Patch Set 4:

(6 comments)

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

Line 1094:       << "or execution summary.";
> indentation
Done


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

Line 220:   Status CheckProfileAccess(const std::string& user);
> const function?
removed


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

Line 636: Status ImpalaServer::QueryStateRecord::CheckProfileAccess(const string& user)
{
> What do you think of moving this into auth-util.h like the other function s
Done


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

Line 639:     Status CheckProfileAccess(const std::string& user);
> const fn
removed


http://gerrit.cloudera.org:8080/#/c/7064/4/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:

Line 162:   private boolean enablePrivChecks_ = true;
> I was hoping we could remove this now because it should have the same effec
That was my initial intention but the way it is used in PartitionSet.java made me change my
mind. There, we explicitly disable priv req registration, so it felt weird to make a call
to setMaskPrivChecks.


http://gerrit.cloudera.org:8080/#/c/7064/4/shell/impala_shell.py
File shell/impala_shell.py:

Line 515:       print_to_stderr(e)
> are these changes for testing?
Without this change, we would always get a "Could not retrieve summary" error (L518) instead
of an auth error.


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message