----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29238/#review65823 -----------------------------------------------------------
sentry-binding/sentry-binding-solr/pom.xml <https://reviews.apache.org/r/29238/#comment109071> Why can this no longer just be in test scope? sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SearchDBProviderBackend.java <https://reviews.apache.org/r/29238/#comment109062> This doesn't seem like the correct location; all the other ProviderBackends are under /sentry-provider sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SearchDBProviderBackend.java <https://reviews.apache.org/r/29238/#comment109066> On the name of the class.. Is there anything that is specific to Search here? Also, DB refers to the service backed by the DB? Should that be named based on Service rather than DB, since there is also a DB model? sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SearchDBProviderBackend.java <https://reviews.apache.org/r/29238/#comment109064> final? sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SearchDBProviderBackend.java <https://reviews.apache.org/r/29238/#comment109069> We don't use the active role set here at all? sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SearchDBProviderBackend.java <https://reviews.apache.org/r/29238/#comment109067> we don't validate the policy or throw an exception saying we aren't validating? sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryRetryClient.java <https://reviews.apache.org/r/29238/#comment109079> do we care that this client is completely synchronous? Performance problem? sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryRetryClient.java <https://reviews.apache.org/r/29238/#comment109075> this isn't accureate, we don't renew the client if it's already been renewed. sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryRetryClient.java <https://reviews.apache.org/r/29238/#comment109074> Error connecting to the sentry service. Renewing the client and trying again sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryRetryClient.java <https://reviews.apache.org/r/29238/#comment109080> I still don't get why this is called requestorUserName. I want to get the roles for a certain user, whether that is the requestor or not seems orthogonal and something that should be handled by the authenticaiton layer. sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/conf/SolrAuthzConf.java <https://reviews.apache.org/r/29238/#comment109072> isn't this the service name, not the server name? sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java <https://reviews.apache.org/r/29238/#comment109060> Can we add some documentation here? What is the requestor param for? - Gregory Chanan On Dec. 19, 2014, 5:54 a.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29238/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2014, 5:54 a.m.) > > > Review request for sentry, Gregory Chanan and Vamsee Yarlagadda. > > > Repository: sentry > > > Description > ------- > > Currently Solr does support index-level security via sentry with file as > backend privilege store. Using file as the backend store is simple, but there > has some disadvantages as followings: > 1. Can't satisfied with the needs of dynamically add, delete and update > permissions > 2. Can't be centrally managed and difficult to maintain > According to the above disadvantages, The Solr Sentry plug-in integration > with DB store is demanded. The Hive Sentry plug-in has already integration > with DB store, but the Hive authorization model is different from the Solr > authorization model.So this new feature depends on the generic authorization > model(SENTRY-398). > > > Diffs > ----- > > sentry-binding/sentry-binding-solr/pom.xml 2dfc933 > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SearchDBProviderBackend.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java > faf862f > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrSentryRetryClient.java > PRE-CREATION > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/conf/SolrAuthzConf.java > 227f75e > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimpleCacheProviderBackend.java > 4b98447 > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java > ddb9cf9 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/SimpleDBProviderBackend.java > ea8eb79 > > sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java > fa5ab69 > > Diff: https://reviews.apache.org/r/29238/diff/ > > > Testing > ------- > > > Thanks, > > shen guoquan > >
