----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34002/#review83297 -----------------------------------------------------------
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java <https://reviews.apache.org/r/34002/#comment134220> This interface seems strange for a couple of reasons: 1) Why does it take a resource if it isn't used? Is this supposed to be hadoopConf.addResource(resource)? Or is resource only there to have a consistent constructor argument list with other mapping services? If the later, that seems problematic because it's not obvious that needs to be the case; for example, I think using the NoGroupService would result in an exception rather than just having a service with no groups. Perhaps we should make the base an abstract class with the required constructor(s) defined? 2) Why do you build the configuration this way, instead of just using the passed in configuration? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java <https://reviews.apache.org/r/34002/#comment134215> does performance matter here? use double checked locking? - Gregory Chanan On May 11, 2015, 5:46 p.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34002/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 5:46 p.m.) > > > Review request for sentry, Colin Ma and Gregory Chanan. > > > Bugs: SENTRY-695 > https://issues.apache.org/jira/browse/SENTRY-695 > > > Repository: sentry > > > Description > ------- > > Sentry service doesn't read default configuration resource. As a result any > group mapping related properties (eg ldap properties etc) are not loaded from > hadoop config files like core-site.xml. These properties have to be > duplicated in sentry-site as well. When Sentry service is using underlying > Hadoop group mapping, it should read the hadoop configuration which might > include group mapping specific properties. > > This patch is enabling Sentry's Hadoop Group mapping to load the hadoop > configuration files if available. Also the group mapping service instance is > Sentry is cached so that we don't have to instantiate it for every RPC > request. > > > Diffs > ----- > > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java > 3347ffc > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 30792f3 > > Diff: https://reviews.apache.org/r/34002/diff/ > > > Testing > ------- > > > Thanks, > > Prasad Mujumdar > >
