You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2018/11/05 21:34:39 UTC

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11880


Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 489 insertions(+), 151 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11880/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 706 insertions(+), 151 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11880/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 6: Code-Review+2

(2 comments)

just minor suggestions from my end to clarify the tests.

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@242
PS3, Line 242: server=server1->d
> It's just a good additional test case usually when dealing with case. I can
its fine to keep it. I just saw it used in one place, to retrieve the role from the policy, as opposed to the more comprehensive treatment that upper-case gets.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@291
PS3, Line 291: 
> If we just used one privilege, we wouldn't know if the user was assigned to
ok, add a brief comment to clarify the purpose of these differences.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 00:28:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 706 insertions(+), 151 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11880/3
-- 
To view, visit http://gerrit.cloudera.org:8080/11880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 6:

Rebased.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Nov 2018 16:24:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11880/2/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/2/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@152
PS2, Line 152:       addSentryPrincipalPrivileges(ctx.type_, ctx.sentryService_, principalName, "functional",
line too long (94 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 05:45:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1362/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 04:13:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1282/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Nov 2018 22:08:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 793 insertions(+), 161 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11880/7
-- 
To view, visit http://gerrit.cloudera.org:8080/11880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 03:31:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1328/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 06:30:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@94
PS3, Line 94: l
> nit: use consistent spacing for these.
Done


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@100
PS3, Line 100: 
> this (resetVersions) seems to have caused issues yet all of the tests here 
Yeah good idea. Tests updated.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@242
PS3, Line 242: server=server1->d
> what's the point of this one?
It's just a good additional test case usually when dealing with case. I can remove it if you think it's not necessary.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@261
PS3, Line 261: 
> just use the upperCaseRoleName?
Oops. Yeah this can just be upperCaseRoleName.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@266
PS3, Line 266:   }
> what is the purpose of this refresh? the operation before is expected to fa
Yeah I guess you're right. Done.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@291
PS3, Line 291: 
> these are all different users, so why the different privs?
If we just used one privilege, we wouldn't know if the user was assigned to the right privilege. There could be a bug where there was only one privilege and it was used by 3 different users as opposed to having 3 privileges (even though they are the same). Having 3 separate privileges ensures that there are 3 different privileges stored in the catalog.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@295
PS3, Line 295: atch (Exception e) {
> expecting this to be about users.
Typo. Done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Nov 2018 15:37:14 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1342/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Nov 2018 16:16:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 3:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@94
PS3, Line 94:  
nit: use consistent spacing for these.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@100
PS3, Line 100: false
this (resetVersions) seems to have caused issues yet all of the tests here consider only the false case. worth adding tests when set to true? from looking at that code, checking that updates get a version change (on reset) would test priv name construction and its lookup at the least.


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@242
PS3, Line 242: mixedCaseRoleName
what's the point of this one?


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@261
PS3, Line 261: lowerCaseRoleName.toUpperCase()
just use the upperCaseRoleName?


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@266
PS3, Line 266:     SentryProxy.refreshSentryAuthorization(catalog, sentryService_, USER, false);
what is the purpose of this refresh? the operation before is expected to fail (L261).


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@291
PS3, Line 291: "functional_kudu"
these are all different users, so why the different privs?


http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@295
PS3, Line 295: Impala stores the role name in case insensitive way.
expecting this to be about users.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Nov 2018 00:28:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 7: Code-Review+2

(1 comment)

Carry Vuk's +2.

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/3/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@291
PS3, Line 291: 
> ok, add a brief comment to clarify the purpose of these differences.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 03:30:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 791 insertions(+), 161 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11880/6
-- 
To view, visit http://gerrit.cloudera.org:8080/11880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11880/1/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/1/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@89
PS1, Line 89:   public void testRefreshRolePrivileges() throws ImpalaException {
this is a great improvement to make the proxy easier to test. currently, there seems to be a fair bit of boiler-plate to setup state on the catalog, on sentry, then check. perhaps this can be abstracted a bit, something like this:

setCatalogRoles( [ {ADD, updateRole, ["functional"]}, 
                               {ADD, removeRole, ["functional"]} ])
setSentryStateRoles( [ {ADD, addRole, ["functional"]},
                             {ADD, updateRole, ["functional", 
                                                             "functional_kudu"]},
                             {DROP, removeRole}])

refresh()

checkCatalogRoles([{updateRole, ["functional", 
                                                         "functional_kudu"]},
                                  {addRole, ["functional"]}])

representation can differ a bit, but the idea is to make it easier to try different combinations. examples:
- explicit test for empty case
- explicit tests for add, delete, update
- non-additive updates
- non-empty state to empty state to non-empty state
- sentry drop of a role not in the catalog
- any limits for number of roles or privs?

this operation is basically trying to merge two collections, so coverage of these various states would be useful.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 00:51:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11880/1/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/1/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@89
PS1, Line 89:       String[] principalNames = new String[3];
> this is a great improvement to make the proxy easier to test. currently, th
Thanks for the feedback. It's a great list. I have revamped the test to cover more various states as you suggested, except the last one since there's no limit.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 05:45:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1327/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 06:21:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11880/2/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
File fe/src/test/java/org/apache/impala/util/SentryProxyTest.java:

http://gerrit.cloudera.org:8080/#/c/11880/2/fe/src/test/java/org/apache/impala/util/SentryProxyTest.java@152
PS2, Line 152:       addSentryPrincipalPrivileges(ctx.type_, ctx.sentryService_, principalName,
> line too long (94 > 90)
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 08 Nov 2018 05:47:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Reviewed-on: http://gerrit.cloudera.org:8080/11880
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 793 insertions(+), 161 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................

IMPALA-7764: Improve SentryProxy test coverage

This patch refactors the SentryProxy to be more testable as well as
adding tests for SentryProxy to improve the test coverage.

Testing:
- Added a new FE test for SentryProxy
- Ran all FE tests
- Ran all E2E authorization tests

Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
---
M fe/src/main/java/org/apache/impala/util/SentryProxy.java
A fe/src/test/java/org/apache/impala/util/SentryProxyTest.java
2 files changed, 779 insertions(+), 151 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/11880/5
-- 
To view, visit http://gerrit.cloudera.org:8080/11880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1343/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Nov 2018 17:05:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3457/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 03:31:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Adam Holley (Code Review)" <ge...@cloudera.org>.
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 1: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 05 Nov 2018 22:16:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7764: Improve SentryProxy test coverage

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11880 )

Change subject: IMPALA-7764: Improve SentryProxy test coverage
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1190638283f051cc37fda7ea5dd0c5e8a70d40db
Gerrit-Change-Number: 11880
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Adam Holley <ah...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 07:40:51 +0000
Gerrit-HasComments: No