> On Feb. 19, 2015, 6:09 a.m., Lenni Kuff wrote: > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java, > > line 64 > > <https://reviews.apache.org/r/29239/diff/3/?file=839746#file839746line64> > > > > consider naming these: > > > > GROUP=admin > > ROLE=admin_role > > COLLECTION_NAME=admin_collection > > > > it might make it easier to debug if there is a test failure (otherwise > > there are a lot of things named "admin")
Thanks Lenni for your good advice. I will fix it. > On Feb. 19, 2015, 6:09 a.m., Lenni Kuff wrote: > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java, > > line 98 > > <https://reviews.apache.org/r/29239/diff/3/?file=839746#file839746line98> > > > > If a new property is set, it looks like it is important to unset it. > > Perhaps split this out in to two functions - setSystemProperties / > > unsetSystemProperties and add them next to eachother (line number wise) so > > it is very easy to see what is and what is not being set. Also comment that > > it is important to unset these props. > > > > Question - why set these as system properties rather than in the conf > > object? I will put these properties in two new functions. Thanks for your suggestion. -->why set these as system properties rather than in the conf object? Because the solr binding will get these from properties from the system properties not from the configuration. > On Feb. 19, 2015, 6:09 a.m., Lenni Kuff wrote: > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java, > > line 256 > > <https://reviews.apache.org/r/29239/diff/3/?file=839746#file839746line256> > > > > thanks for commenting the test cases, this makes it easy to see what's > > going on without reading all the code. Thanks Lenni. I will fix it - shen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29239/#review73090 ----------------------------------------------------------- On Feb. 19, 2015, 5:33 a.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29239/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2015, 5:33 a.m.) > > > Review request for sentry, Xiaomeng Huang, Colin Ma, Dapeng Sun, Gregory > Chanan, Lenni Kuff, 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: > > Can't satisfied with the needs of dynamically add, delete and update > permissions > 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-tests/sentry-tests-solr/pom.xml dfc3792 > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/AbstractSolrSentryTestBase.java > 6d2550b > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrAdminOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrDocLevelOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrQueryOperations.java > PRE-CREATION > > sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/TestSolrUpdateOperations.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29239/diff/ > > > Testing > ------- > > > Thanks, > > shen guoquan > >
