----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29239/#review73090 -----------------------------------------------------------
Ship it! sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java <https://reviews.apache.org/r/29239/#comment119290> 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") sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java <https://reviews.apache.org/r/29239/#comment119292> 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? sentry-tests/sentry-tests-solr/src/test/java/org/apache/sentry/tests/e2e/solr/db/integration/AbstractSolrSentryTestWithDbProvider.java <https://reviews.apache.org/r/29239/#comment119291> thanks for commenting the test cases, this makes it easy to see what's going on without reading all the code. - Lenni Kuff 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 > >
