> On 二月 12, 2015, 6:29 p.m., Lenni Kuff wrote: > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java, > > line 75 > > <https://reviews.apache.org/r/30068/diff/3/?file=841296#file841296line75> > > > > Add a comment on which user this is expected to be
Thanks Lenni. I will fix it. > On 二月 12, 2015, 6:29 p.m., Lenni Kuff wrote: > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java, > > line 226 > > <https://reviews.apache.org/r/30068/diff/3/?file=841296#file841296line226> > > > > Maybe get rid of "isSync" and just return: > > > > providerBackend instanceof SearchProviderBackend; > > ? Good advice. Thanks Lenni. I will change it. > On 二月 12, 2015, 6:29 p.m., Lenni Kuff wrote: > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java, > > line 67 > > <https://reviews.apache.org/r/30068/diff/3/?file=841296#file841296line67> > > > > Can you expand on exactly what "sync" means? Maybe rename to > > "isSyncEnabled"? The Solr binding may be use the policy file as the backend. If the backend is file store, It doesn't need to sync. When the solr integration the db store, it need to sync with the sentry service > On 二月 12, 2015, 6:29 p.m., Lenni Kuff wrote: > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java, > > line 250 > > <https://reviews.apache.org/r/30068/diff/3/?file=841296#file841296line250> > > > > Are you sure that the only time you make it to this block is when the > > connection could not be obtained? Yeah, when the client was successfully created, there is only the SentryUserException throwed. If the other exception throw, there must be that the client was created failed. > On 二月 12, 2015, 6:29 p.m., Lenni Kuff wrote: > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRequestHandlerUtil.java, > > line 61 > > <https://reviews.apache.org/r/30068/diff/3/?file=841297#file841297line61> > > > > Maybe I am missing something, but I don't see any code to handle the > > "rename" case. Hi Lenni. Thanks for your help reviewing my patch about solr integration with db store. --> but I don't see any code to handle the "rename" case Currently there is one operation lead to the metadata been changed when solr is running solrcloud mode. This operation is deleting a collection, and the collection API is /admin/collections?action=DELETE&name={collectionName}. when the collection has been deleted in solr, the privileges related to the collection wsa needed to drop in sentry. According to the rename operation, I found when the solr running in the solrcloud mode, the collection name will not been changed with this Operation. For exmaple, the solrcloud has one collection called collection1, and the collection has two cores: collection1_core_1, collection1_core_2. The rename operation just change the name of core in the collection. It will not change the collection name. So it doesn not need to sync with sentry. When the solr running in the non-solrcloud mode, the binding is not currently supported. So the there is no code related the rename operation. - shen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30068/#review71541 ----------------------------------------------------------- On 一月 30, 2015, 5:40 a.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30068/ > ----------------------------------------------------------- > > (Updated 一月 30, 2015, 5:40 a.m.) > > > Review request for sentry, Xiaomeng Huang, Colin Ma, Dapeng Sun, Gregory > Chanan, and Vamsee Yarlagadda. > > > Repository: sentry > > > Description > ------- > > Solr needs to sync collection privileges with Sentry when the metadata has > changed. For example, when the collection has been deleted, the privileges > related to the collection were needed to drop > > > Diffs > ----- > > > sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java > faf862f > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/SecureRequestHandlerUtil.java > 8b46388 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java > 20d9626 > > sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java > d44b228 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrAdminOperations.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/30068/diff/ > > > Testing > ------- > > All tests have passed through the local jenkins environment > > > Thanks, > > shen guoquan > >
