You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by kalyan kumar kalvagadda <kk...@cloudera.com> on 2017/07/14 19:49:09 UTC
Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/
-----------------------------------------------------------
Review request for sentry and Sergio Pena.
Bugs: SENTRY-1839
https://issues.apache.org/jira/browse/SENTRY-1839
Repository: sentry
Description
-------
Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
* AuthorizingObjectStoreBase.java
* HiveAuthzBindingHookBase.java
* MetastoreAuthzBindingBase.java
* SentryHiveMetaStoreClient.java
* SentryMetastorePostEventListenerBase.java
Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
packages so that they can have a separate implementation.
Diffs
-----
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
Diff: https://reviews.apache.org/r/60880/diff/1/
Testing
-------
Thanks,
kalyan kumar kalvagadda
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On July 17, 2017, 5:02 p.m., Colm O hEigeartaigh wrote:
> > I think more work could be done on having a common base. Just to take one example, the only substantive difference between the two SentryFilterDDLTask implementations is:
> >
> > < return HiveAuthzBindingHookBaseV2.filterShowColumns(getHiveAuthzBinding(),
> > ---
> > > return HiveAuthzBindingHookBase.filterShowColumns(getHiveAuthzBinding(),
> >
> > Yet there's quite a lot of code duplicated in the two implementations. Is it possible to try to keep the base class in the common module, and just abstract the differences that can be subclassed in the individual modules?
>
> kalyan kumar kalvagadda wrote:
> I agree, there can be better way to do it.
> I don't think we need to support multiple versions of hive in sentry 2.0.0. When sentry 2.0.0 is released it should be working with Hive 2.1.1 and need not be backword compatible with older version of Hive.
> I took this approach not to effect the current development going on on sentry-ha. Once support fot Hive 2.1.1 is stable enough we can remove the support for Hive 1.1.0.
>
> I will send an email to the sentry community asking about feedback on this approach.
>
> Colm O hEigeartaigh wrote:
> I get an error when trying to build the v2 binding - it complains that DefaultHiveAuthorizationTranslator is missing.
Colm,
With the changes done for SENTRY-1849 we haved fixed dependencies that were wrong. This brings in need to make additional code changes to build for hive-auth2 profile.
These changes are in the common code that is used by both senty-binding-hive and sentry-binding-hive-v2 packages. Current change set is just first step towards that.
I broke the effort for making the code properly build for V2 in to threee parts
1. SENTRY-1839 Duplicate file in sentry-binding-hive-common package
2. SENTRY-1843 Add sentry-binding-hive-follower-v2 package to support Hive 2.1.1
3. SENTRY-1847 Fix the compilation issues while building with hive-auth2 profile
I did not want to do all of it in a single commit as it would be difficult to track and review the code changes.
Once above three jira's are fixed code would be build for hive-auth2.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review180692
-----------------------------------------------------------
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On July 17, 2017, 5:02 p.m., Colm O hEigeartaigh wrote:
> > I think more work could be done on having a common base. Just to take one example, the only substantive difference between the two SentryFilterDDLTask implementations is:
> >
> > < return HiveAuthzBindingHookBaseV2.filterShowColumns(getHiveAuthzBinding(),
> > ---
> > > return HiveAuthzBindingHookBase.filterShowColumns(getHiveAuthzBinding(),
> >
> > Yet there's quite a lot of code duplicated in the two implementations. Is it possible to try to keep the base class in the common module, and just abstract the differences that can be subclassed in the individual modules?
I agree, there can be better way to do it.
I don't think we need to support multiple versions of hive in sentry 2.0.0. When sentry 2.0.0 is released it should be working with Hive 2.1.1 and need not be backword compatible with older version of Hive.
I took this approach not to effect the current development going on on sentry-ha. Once support fot Hive 2.1.1 is stable enough we can remove the support for Hive 1.1.0.
I will send an email to the sentry community asking about feedback on this approach.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review180692
-----------------------------------------------------------
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by Colm O hEigeartaigh <co...@apache.org>.
> On July 17, 2017, 5:02 p.m., Colm O hEigeartaigh wrote:
> > I think more work could be done on having a common base. Just to take one example, the only substantive difference between the two SentryFilterDDLTask implementations is:
> >
> > < return HiveAuthzBindingHookBaseV2.filterShowColumns(getHiveAuthzBinding(),
> > ---
> > > return HiveAuthzBindingHookBase.filterShowColumns(getHiveAuthzBinding(),
> >
> > Yet there's quite a lot of code duplicated in the two implementations. Is it possible to try to keep the base class in the common module, and just abstract the differences that can be subclassed in the individual modules?
>
> kalyan kumar kalvagadda wrote:
> I agree, there can be better way to do it.
> I don't think we need to support multiple versions of hive in sentry 2.0.0. When sentry 2.0.0 is released it should be working with Hive 2.1.1 and need not be backword compatible with older version of Hive.
> I took this approach not to effect the current development going on on sentry-ha. Once support fot Hive 2.1.1 is stable enough we can remove the support for Hive 1.1.0.
>
> I will send an email to the sentry community asking about feedback on this approach.
I get an error when trying to build the v2 binding - it complains that DefaultHiveAuthorizationTranslator is missing.
- Colm
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review180692
-----------------------------------------------------------
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review180692
-----------------------------------------------------------
I think more work could be done on having a common base. Just to take one example, the only substantive difference between the two SentryFilterDDLTask implementations is:
< return HiveAuthzBindingHookBaseV2.filterShowColumns(getHiveAuthzBinding(),
---
> return HiveAuthzBindingHookBase.filterShowColumns(getHiveAuthzBinding(),
Yet there's quite a lot of code duplicated in the two implementations. Is it possible to try to keep the base class in the common module, and just abstract the differences that can be subclassed in the individual modules?
- Colm O hEigeartaigh
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by kalyan kumar kalvagadda <kk...@cloudera.com>.
> On July 17, 2017, 9:05 a.m., Colm O hEigeartaigh wrote:
> > Does the SentryFilterDDLTask.java need to be in the org.apache.hadoop.hive namespace?
That is how it was even before. I did not want to change it.
- kalyan kumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review180662
-----------------------------------------------------------
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review180662
-----------------------------------------------------------
Does the SentryFilterDDLTask.java need to be in the org.apache.hadoop.hive namespace?
- Colm O hEigeartaigh
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>
Re: Review Request 60880: SENTRY-1839 Duplicate file in
sentry-binding-hive-common package
Posted by Colm O hEigeartaigh <co...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60880/#review181667
-----------------------------------------------------------
Ship it!
Ship It!
- Colm O hEigeartaigh
On July 14, 2017, 7:49 p.m., kalyan kumar kalvagadda wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60880/
> -----------------------------------------------------------
>
> (Updated July 14, 2017, 7:49 p.m.)
>
>
> Review request for sentry and Sergio Pena.
>
>
> Bugs: SENTRY-1839
> https://issues.apache.org/jira/browse/SENTRY-1839
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Originally sentry-binding-hive-common was created by extracting common classes binding-hive-v1 and binding-hive-v2(hive 2.0.0). With the changes done to hive interface in 2.1.1 that is not the case. Below listed files need different implementation.
>
> * AuthorizingObjectStoreBase.java
> * HiveAuthzBindingHookBase.java
> * MetastoreAuthzBindingBase.java
> * SentryHiveMetaStoreClient.java
> * SentryMetastorePostEventListenerBase.java
> Idea to copy a copy of these file to sentry-binding-hive and sentry-binding-hive-v2
> packages so that they can have a separate implementation.
>
>
> Diffs
> -----
>
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java e257360
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHookBase.java c4daea1
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java 8a5085b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java 196bd2b
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java 3e2a9ea
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java b5df287
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java 2a0a5b8
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java 2abdd53
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingHookV2.java 018ebf3
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/HiveAuthzBindingSessionHookV2.java b95bf60
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/SentryAuthorizerFactory.java 485ac43
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/AuthorizingObjectStoreV2.java 913bd00
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/MetastoreAuthzBindingV2.java cfef1a7
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryHiveMetaStoreClientV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerBaseV2.java PRE-CREATION
> sentry-binding/sentry-binding-hive-v2/src/main/java/org/apache/sentry/binding/hive/v2/metastore/SentryMetastorePostEventListenerV2.java f1e981f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryFilterDDLTask.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java 3f06cae
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java 34683f4
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingSessionHook.java 6d9150f
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStore.java b2377d8
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/AuthorizingObjectStoreBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryHiveMetaStoreClient.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListenerBase.java PRE-CREATION
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestURI.java b920d49
> sentry-tests/sentry-tests-hive-v2/src/test/java/org/apache/sentry/tests/e2e/minisentry/InternalSentrySrv.java 62bcf62
>
>
> Diff: https://reviews.apache.org/r/60880/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> kalyan kumar kalvagadda
>
>