[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The fourth part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Cherry-picks: not for 2.x Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Reviewed-on: http://gerrit.cloudera.org:8080/10442 Tested-by: Impala Public Jenkins Reviewed-by: Vuk Ercegovac --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 279 insertions(+), 57 deletions(-) Approvals: Impala Public Jenkins: Verified Vuk Ercegovac: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 6 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 5: Seems ok to go ahead and merge. There haven't been any related commits that have gone in since you ran the tests. -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 18 Jun 2018 22:38:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 18 Jun 2018 05:12:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 18:43:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2684/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 15 Jun 2018 15:20:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The fourth part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Cherry-picks: not for 2.x Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 279 insertions(+), 57 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10442/5 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 5 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 4: I'm fine with it.. one more small change needed to comment style -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 14 Jun 2018 23:14:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 4: I think this should be ok to merge whenever Vuk is happy with it. -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 14 Jun 2018 22:29:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 4: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@946 PS3, Line 946: ctional", : "alltypes", privilege)); > This is the pattern we've been using in these tests to be explicit for each hmm.. ok, guess it takes a bit of getting used to http://gerrit.cloudera.org:8080/#/c/10442/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@919 PS4, Line 919: // use /** style comments for methods. -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 13 Jun 2018 23:53:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 4: Code-Review+1 Carry +1 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 12 Jun 2018 21:48:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The fourth part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Cherry-picks: not for 2.x Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 277 insertions(+), 57 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10442/4 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 3: Code-Review+1 (6 comments) updated. carry +1 http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@918 PS3, Line 918: @Test > perhaps add a comment explaining what's being tested... from a first glance Done http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@922 PS3, Line 922: > consistent spacing (see comment below) Done http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@938 PS3, Line 938: > and several more places below Done http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@946 PS3, Line 946: allExcept( : TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT) > factor out (repeated 3 times in this block, so it adds too much noise). This is the pattern we've been using in these tests to be explicit for each test on the privileges required. It adds text, but reduces the need to reference what the variable or constant means in a different part of the code. http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@952 PS3, Line 952: ng[]{"id"}, > explain what is being tested here (missing why "id" is special) Done http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1181 PS3, Line 1181: > use consistent spacing for this... from a brief look, seems like no space i Done -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 12 Jun 2018 21:48:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@918 PS3, Line 918: @Test perhaps add a comment explaining what's being tested... from a first glance, it looks like certain privs can see certain fields, and others cannot? if there are docs or jiras that explain this more, pls reference them here. http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@922 PS3, Line 922: consistent spacing (see comment below) http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@938 PS3, Line 938: and several more places below http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@946 PS3, Line 946: allExcept( : TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT) factor out (repeated 3 times in this block, so it adds too much noise). http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@952 PS3, Line 952: ng[]{"id"}, explain what is being tested here (missing why "id" is special) http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1181 PS3, Line 1181: use consistent spacing for this... from a brief look, seems like no space is the preference. -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 08 Jun 2018 22:02:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 3: Code-Review+1 Vuk, can you take a look at it? -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 08 Jun 2018 16:51:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The fourth part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Cherry-picks: not for 2.x Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 251 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10442/3 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Fredy Wijaya has removed a vote on this change. Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Removed Code-Review+1 by Fredy Wijaya -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 2 Gerrit-Owner: Adam HolleyGerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Wed, 23 May 2018 14:30:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 1: (11 comments) Taking this over since Adam is out. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@104 PS1, Line 104: private static final String[] ALLTYPES_COLUMNS = (String []) ArrayUtils.addAll( > nit: String[] Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@923 PS1, Line 923: .ok(onServer(TPrivilegeLevel.INSERT)) > We need to add REFRESH since VIEW_METADATA consists of SELECT, INSERT, and Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@927 PS1, Line 927: .error(accessError("functional")); > Missing check .error(accessError("functional"), onDatabase...) Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@933 PS1, Line 933: TTableName tableName = new TTableName("functional","alltypes"); > nit: space after comma Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@935 PS1, Line 935: authorize("describe functional.alltypes") > describe uses ANY privilege, we need more test cases with all other Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@961 PS1, Line 961: String [] locationString = new String[]{"Location:"}; > nit: no space for array declaration Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@989 PS1, Line 989: tableName = new TTableName("functional","alltypes_view"); > nit: space after comma Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1013 PS1, Line 1013: tableName = new TTableName("functional","alltypes_view"); > nit: space after comma Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1018 PS1, Line 1018: viewStrings); > join L1018 with L1017 Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1040 PS1, Line 1040: authorize("describe functional.allcomplextypes.int_struct_col") > describe uses ANY privilege, we need more test cases with all other Done http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1183 PS1, Line 1183: String [] requiredStrings, String [] excludedStrings, > nit: fix indentation Done -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 1 Gerrit-Owner: Adam HolleyGerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Wed, 23 May 2018 14:29:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@923 PS1, Line 923: authzTest.ok(onServer(privilege)) > We need to add REFRESH since VIEW_METADATA consists of SELECT, INSERT, and I think this should be changed so it’s only select. Different Jira to fix though. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@935 PS1, Line 935: TTableName tableName = new TTableName("functional", "alltypes"); > describe uses ANY privilege, we need more test cases with all other Same as for database. I think only select should allow describe. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1018 PS1, Line 1018: .okDescribe(tableName, style, checkStrings, null, onTable("functional", > join L1018 with L1017 These need to be separate since then view text does not show for insert. -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 2 Gerrit-Owner: Adam HolleyGerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Wed, 23 May 2018 07:29:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The fourth part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Cherry-picks: not for 2.x Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 251 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10442/2 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 2 Gerrit-Owner: Adam HolleyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 ) Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. Patch Set 1: (11 comments) http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@104 PS1, Line 104: private static final String[] ALLTYPES_COLUMNS = (String []) ArrayUtils.addAll( nit: String[] http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@923 PS1, Line 923: .ok(onServer(TPrivilegeLevel.INSERT)) We need to add REFRESH since VIEW_METADATA consists of SELECT, INSERT, and REFRESH http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@927 PS1, Line 927: .error(accessError("functional")); Missing check .error(accessError("functional"), onDatabase...) http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@933 PS1, Line 933: TTableName tableName = new TTableName("functional","alltypes"); nit: space after comma http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@935 PS1, Line 935: authorize("describe functional.alltypes") describe uses ANY privilege, we need more test cases with all other privileges. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@961 PS1, Line 961: String [] locationString = new String[]{"Location:"}; nit: no space for array declaration http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@989 PS1, Line 989: tableName = new TTableName("functional","alltypes_view"); nit: space after comma http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1013 PS1, Line 1013: tableName = new TTableName("functional","alltypes_view"); nit: space after comma http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1018 PS1, Line 1018: viewStrings); join L1018 with L1017 http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1040 PS1, Line 1040: authorize("describe functional.allcomplextypes.int_struct_col") describe uses ANY privilege, we need more test cases with all other privileges. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1183 PS1, Line 1183: String [] requiredStrings, String [] excludedStrings, nit: fix indentation -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 1 Gerrit-Owner: Adam HolleyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Tue, 22 May 2018 23:11:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6802 (part 4): Clean up authorization tests
Adam Holley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10442 Change subject: IMPALA-6802 (part 4): Clean up authorization tests .. IMPALA-6802 (part 4): Clean up authorization tests The third part of this patch is to rewrite the following authorization tests: - describe Testing: - Added new authorization tests - Ran all front-end tests Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Cherry-picks: not for 2.x --- M fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java 1 file changed, 262 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/10442/1 -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 1 Gerrit-Owner: Adam HolleyGerrit-Reviewer: Alex Behm Gerrit-Reviewer: Fredy Wijaya