> 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 > >
