You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Sravya Tirukkovalur <sr...@cloudera.com> on 2014/06/02 19:01:26 UTC
Re: 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/#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
>
>