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/06/06 10:12:45 UTC
Review Request 22301: SENTRY-259: Implement Hive metastore plugin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22301/
-----------------------------------------------------------
Review request for sentry, Arun Suresh, Brock Noland, and Sravya Tirukkovalur.
Bugs: SENTRY-259
https://issues.apache.org/jira/browse/SENTRY-259
Repository: sentry
Description
-------
- Basic metastore binding via pre-even listener hooks. Uses the same privilege model as hive.
- Test framework support for starting a metastore server
Diffs
-----
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 54c9a41
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 7b7bf8e
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java 79ca387
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 84223a9
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 835e547
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java 39d411e
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSentryOnFailureHookLoading.java cae270b
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 184c066
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 19ff6cf
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalMetastoreServer.java PRE-CREATION
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java PRE-CREATION
Diff: https://reviews.apache.org/r/22301/diff/
Testing
-------
basic e2e tests with metastore server. Additional metastore test cases will be added via a separate patch.
Thanks,
Prasad Mujumdar
Re: Review Request 22301: SENTRY-259: Implement Hive metastore plugin
Posted by Prasad Mujumdar <pr...@cloudera.com>.
> On June 6, 2014, 7:58 p.m., Jarek Cecho wrote:
> > Couple of high level notes.
The reverts are due to the branch being out of sync with master. I will close this one and different review request. Addressed the feedback, except a followup ticket for the kerberos related ToDo. will log ticket later.
> On June 6, 2014, 7:58 p.m., Jarek Cecho wrote:
> > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java, lines 274-276
> > <https://reviews.apache.org/r/22301/diff/1/?file=604693#file604693line274>
> >
> > Isn't this equivalent with:
> >
> > return InvalidOperationException(e.getMessage(), e)
Unfortunately, this is a this exception is defined in Metastore thrift IDL, and hence comming from the generated java code. It has just one constructor which is why the workaround.
- Prasad
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22301/#review44954
-----------------------------------------------------------
On June 6, 2014, 8:12 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22301/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 8:12 a.m.)
>
>
> Review request for sentry, Arun Suresh, Brock Noland, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-259
> https://issues.apache.org/jira/browse/SENTRY-259
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Basic metastore binding via pre-even listener hooks. Uses the same privilege model as hive.
> - Test framework support for starting a metastore server
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 54c9a41
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 7b7bf8e
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java 79ca387
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 84223a9
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 835e547
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java 39d411e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSentryOnFailureHookLoading.java cae270b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 184c066
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 19ff6cf
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalMetastoreServer.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22301/diff/
>
>
> Testing
> -------
>
> basic e2e tests with metastore server. Additional metastore test cases will be added via a separate patch.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>
Re: Review Request 22301: SENTRY-259: Implement Hive metastore plugin
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22301/#review44954
-----------------------------------------------------------
Couple of high level notes.
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
<https://reviews.apache.org/r/22301/#comment79544>
I'm wondering if this is an intentional revert of change introduced in SENTRY-217 or just accidental removal?
https://github.com/apache/incubator-sentry/commit/540424d50f84c3ad0bd65370374e3d70c352bb72#diff-1924ec6eed6e56538f96627a4832fb29L416
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java
<https://reviews.apache.org/r/22301/#comment79545>
Let's nuke this comment?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java
<https://reviews.apache.org/r/22301/#comment79546>
Nit: Trailing whitespace
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java
<https://reviews.apache.org/r/22301/#comment79547>
Should we create a follow up JIRA for this one then?
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java
<https://reviews.apache.org/r/22301/#comment79549>
Isn't this equivalent with:
return InvalidOperationException(e.getMessage(), e)
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
<https://reviews.apache.org/r/22301/#comment79550>
Is this intentional revert of SENTRY-255?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
<https://reviews.apache.org/r/22301/#comment79551>
Let's create follow up JIRA?
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalMetastoreServer.java
<https://reviews.apache.org/r/22301/#comment79555>
Can we log this rather then using stderr?
Jarcec
- Jarek Cecho
On June 6, 2014, 8:12 a.m., Prasad Mujumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22301/
> -----------------------------------------------------------
>
> (Updated June 6, 2014, 8:12 a.m.)
>
>
> Review request for sentry, Arun Suresh, Brock Noland, and Sravya Tirukkovalur.
>
>
> Bugs: SENTRY-259
> https://issues.apache.org/jira/browse/SENTRY-259
>
>
> Repository: sentry
>
>
> Description
> -------
>
> - Basic metastore binding via pre-even listener hooks. Uses the same privilege model as hive.
> - Test framework support for starting a metastore server
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 54c9a41
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/conf/HiveAuthzConf.java 7b7bf8e
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBinding.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java 79ca387
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java 84223a9
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java 835e547
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java 39d411e
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestSentryOnFailureHookLoading.java cae270b
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/fs/MiniDFS.java 184c066
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/HiveServerFactory.java 19ff6cf
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/hiveserver/InternalMetastoreServer.java PRE-CREATION
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/metastore/TestMetastoreEndToEnd.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/22301/diff/
>
>
> Testing
> -------
>
> basic e2e tests with metastore server. Additional metastore test cases will be added via a separate patch.
>
>
> Thanks,
>
> Prasad Mujumdar
>
>