You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Prasad Mujumdar <pr...@cloudera.com> on 2014/05/31 12:13:58 UTC
Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/
-----------------------------------------------------------
Review request for sentry and Sravya Tirukkovalur.
Bugs: SENTRY-241
https://issues.apache.org/jira/browse/SENTRY-241
Repository: sentry
Description
-------
The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f1e6247
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
Diff: https://reviews.apache.org/r/22120/diff/
Testing
-------
Added new test cases.
Thanks,
Prasad Mujumdar
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/#review44581
-----------------------------------------------------------
Ship it!
Ship It!
- Sravya Tirukkovalur
On June 3, 2014, 1:11 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22120/
> -----------------------------------------------------------
>
> (Updated June 3, 2014, 1:11 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-241
> https://issues.apache.org/jira/browse/SENTRY-241
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 1f56de7
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
>
> Diff: https://reviews.apache.org/r/22120/diff/
>
>
> Testing
> -------
>
> Added new test cases.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/
-----------------------------------------------------------
(Updated June 3, 2014, 1:11 a.m.)
Review request for sentry and Sravya Tirukkovalur.
Changes
-------
rebase, enhanced the test case.
Bugs: SENTRY-241
https://issues.apache.org/jira/browse/SENTRY-241
Repository: sentry
Description
-------
The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 1f56de7
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
Diff: https://reviews.apache.org/r/22120/diff/
Testing
-------
Added new test cases.
Thanks,
Prasad Mujumdar
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/
-----------------------------------------------------------
(Updated June 2, 2014, 9:57 p.m.)
Review request for sentry and Sravya Tirukkovalur.
Changes
-------
Fix the exception handling in case of the Failures hooks. The exception is now correctly throwing the error.
Bugs: SENTRY-241
https://issues.apache.org/jira/browse/SENTRY-241
Repository: sentry
Description
-------
The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 1012605
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
Diff: https://reviews.apache.org/r/22120/diff/
Testing
-------
Added new test cases.
Thanks,
Prasad Mujumdar
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/#review44518
-----------------------------------------------------------
Ship it!
Ship It!
- Sravya Tirukkovalur
On June 2, 2014, 5:59 p.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22120/
> -----------------------------------------------------------
>
> (Updated June 2, 2014, 5:59 p.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-241
> https://issues.apache.org/jira/browse/SENTRY-241
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f1e6247
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
>
> Diff: https://reviews.apache.org/r/22120/diff/
>
>
> Testing
> -------
>
> Added new test cases.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/
-----------------------------------------------------------
(Updated June 2, 2014, 5:59 p.m.)
Review request for sentry and Sravya Tirukkovalur.
Changes
-------
Updated per review feedback.
Bugs: SENTRY-241
https://issues.apache.org/jira/browse/SENTRY-241
Repository: sentry
Description
-------
The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
Diffs (updated)
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f1e6247
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
Diff: https://reviews.apache.org/r/22120/diff/
Testing
-------
Added new test cases.
Thanks,
Prasad Mujumdar
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/#review44512
-----------------------------------------------------------
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
<https://reviews.apache.org/r/22120/#comment78916>
This is override the method in base class which return Context. Update the two constructors to be in sync.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78917>
Fixed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78918>
yes, removed.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78919>
Done
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78921>
Removed
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78923>
Updated
- Prasad Mujumdar
On May 31, 2014, 10:13 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22120/
> -----------------------------------------------------------
>
> (Updated May 31, 2014, 10:13 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-241
> https://issues.apache.org/jira/browse/SENTRY-241
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f1e6247
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
>
> Diff: https://reviews.apache.org/r/22120/diff/
>
>
> Testing
> -------
>
> Added new test cases.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22120: SENTRY-241: Sentry GrantRevokeTask should fire the
sentry failure look
Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22120/#review44505
-----------------------------------------------------------
Mostly looks good, minor comments below.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java
<https://reviews.apache.org/r/22120/#comment78901>
This can also return void as the other constructor, as context object is visible from the child class( it is protected in parent).
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78902>
context object is available from parent class.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78903>
This is not needed, as there is a cleaner teardown function in the parent class which also stops the server.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78904>
context is already created in @Before
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78906>
load_data role never used?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78909>
Comment seems incorrect, we are trying to create a table here rather than dropping a db.
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78905>
Not needed (See above)
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78907>
load_data role never used?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22120/#comment78910>
Same as above, comment seems incorrect.
- Sravya Tirukkovalur
On May 31, 2014, 10:13 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22120/
> -----------------------------------------------------------
>
> (Updated May 31, 2014, 10:13 a.m.)
>
>
> Review request for sentry and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-241
> https://issues.apache.org/jira/browse/SENTRY-241
>
>
> Repository: sentry
>
>
> Description
> -------
>
> The Hive binding grantRevokeTask should fire the Sentry on-failure hook if configured.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java f1e6247
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java f8c694c
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/AbstractTestWithDbProvider.java c6f1ce2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java e4055a7
>
> Diff: https://reviews.apache.org/r/22120/diff/
>
>
> Testing
> -------
>
> Added new test cases.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>