impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5489: Improve Sentry authorization for Kudu tables
Date Tue, 27 Jun 2017 23:38:24 GMT
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-5489: Improve Sentry authorization for Kudu tables
......................................................................


Patch Set 1:

(2 comments)

Just a couple of nits.

http://gerrit.cloudera.org:8080/#/c/7307/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java:

Line 131:           formatArgs));
Add a test case for all privileges?


http://gerrit.cloudera.org:8080/#/c/7307/1/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

Line 696: create role grant_revoke_test_KUDU
Would it be preferable to create a separate file for Kudu grant revoke tests?
We do that for other tests specific to Kudu IIRC.

The upside is that we can disable kudu tests while running against other platforms and reduce
test time. But the downside is too many test files.

I'm okay with either as long as we've considered both the alternatives.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib12d2b32fa3e142e69bd8b0f24f53f9e5cbf7460
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message