> On 一月 28, 2015, 6:31 a.m., Colin Ma wrote: > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java, > > line 29 > > <https://reviews.apache.org/r/29238/diff/3/?file=816327#file816327line29> > > > > The constant is for configuration, this should be added to > > org.apache.sentry.service.thrift.ServiceConstants.
This configuration was used in the solr component. I think the search model package is a better place than provider-db package > On 一月 28, 2015, 6:31 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java, > > line 65 > > <https://reviews.apache.org/r/29238/diff/3/?file=816332#file816332line65> > > > > Currently, sentry won't retry the operation, if connection is broken, > > just throw the exception. There should be deal with the situation that the sentry service is restarted. This is a simple solution. I will refactor the client when the connection pool was committed. > On 一月 28, 2015, 6:31 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java, > > line 61 > > <https://reviews.apache.org/r/29238/diff/3/?file=816332#file816332line61> > > > > This interface is not necessary. There should be deal with the situation that the sentry service is restarted. This is a simple solution. I will refactor the client when the connection pool was committed. > On 一月 28, 2015, 6:31 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java, > > line 89 > > <https://reviews.apache.org/r/29238/diff/3/?file=816332#file816332line89> > > > > The SearchPolicyServiceClient shouldn't retry the operation currently, > > so renewClient() is not necessary. The reason is above. > On 一月 28, 2015, 6:31 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java, > > line 108 > > <https://reviews.apache.org/r/29238/diff/3/?file=816332#file816332line108> > > > > Confuse on ifExist method, too..... This method is mainly used in the import tool. When the policy file want to import the sentry service, this method is very useful > On 一月 28, 2015, 6:31 a.m., Colin Ma wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java, > > line 49 > > <https://reviews.apache.org/r/29238/diff/3/?file=816332#file816332line49> > > > > The value should be in lowercase as the roleName. The variable name > > should be COMPONENT_TYPE_SOLR, and these kind of constants should be put in > > one class including all component_type, eg: > > COMPONENT_TYPE_SOLR = "solr"; > > COMPONENT_TYPE_SQOOP = "sqoop"; > > ....... Thanks for your advice. I will fix it - shen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29238/#review69962 ----------------------------------------------------------- On 一月 12, 2015, 9:10 a.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29238/ > ----------------------------------------------------------- > > (Updated 一月 12, 2015, 9:10 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/SolrAuthzBinding.java > faf862f > > sentry-core/sentry-core-model-search/src/main/java/org/apache/sentry/core/model/search/SearchConstants.java > 7a39b79 > > sentry-policy/sentry-policy-search/src/main/java/org/apache/sentry/policy/search/SimpleSearchPolicyEngine.java > f428aea > > 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-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchPolicyServiceClient.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SearchProviderBackend.java > PRE-CREATION > > 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 > >
