----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28893/#review64698 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java <https://reviews.apache.org/r/28893/#comment107501> If Sentry is running with HA, there will be a problem to synchronize the local files. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java <https://reviews.apache.org/r/28893/#comment107498> Maybe this exception need to be thrown, or sentry service will get the incorrect authorization data. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java <https://reviews.apache.org/r/28893/#comment107497> 1. There should be a loop to read all reverted records? 2. If there has an exception during revertOis.readInt(), ignore the exception maybe cause the data not correct. The reverted records won't be filtered. This exception should be thrown. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java <https://reviews.apache.org/r/28893/#comment107496> How about using a map to get the Privilege: static Map<String, Privilege> actionPrivilegeMap = ........ { actionPrivilegeMap.put(AccessConstants.ACTION_ALL, ALL); actionPrivilegeMap.put(AccessConstants.CREATE, CREATE); ......... } static Privilege fromTSentryPrivilege(TSentryPrivilege priv) { Privilege result = actionPrivilegeMap.get(Strings.nullToEmpty(priv.getAction())); if (result == null) { result = NONE; } return result; } sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java <https://reviews.apache.org/r/28893/#comment107500> For the name check, maybe we can add test case to test case insentitive. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java <https://reviews.apache.org/r/28893/#comment107499> Maybe add another role named "NEWROLE" to test the case insensitive. - Colin Ma On Dec. 10, 2014, 8:01 a.m., Arun Suresh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28893/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2014, 8:01 a.m.) > > > Review request for sentry, Lenni Kuff, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > The basic motivations for this JIRA is > > 1) Make the Sentry Store pluggable > 2) Provide an implementation of SentryStore that in not dependent on JDO / > Datanucleus or any external data layer. > 3) Separate out the core DataStructures and Persistence logic > > Current status : > 1) It includes a new InMemSentryStore and a ThreadSafeInMemSentryStore > 2) It also includes a persistent FileLoggingSentryStore : This writes out > sentry store operations to a log file and on restart, reads the log file and > populates a backing InMemSentryStore > 3) The tests in sentry-hive-tests can be run against the new store > impementationss : > * add env variable USE_IN_MEM=true to use the in memory store > * add env variable USE_FILE_STORE=true to use the FileLoggingSentryStore > with a backing in memory store > 4) All TestSentrySeviceIntegration tests pass > 5) There is a senparate TestInMemSentryStore that runs most of the same tests > as TestSentryStore > > TODO: > 1) Add support for log compaction > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java > f1e792d > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > 1c68a0f > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/DbSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/FileLoggingSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/InMemSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/PersistentSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > f98e853 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreFactory.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/StoreUtils.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/ThreadSafeSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryMetrics.java > 55bec0b > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 29e3131 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > 993ea46 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestFileLoggingSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestInMemSentryStore.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 8fbe3f4 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStoreToAuthorizable.java > 922cbc2 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryVersion.java > 0add58b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > 4a6cac9 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/SentryServiceIntegrationBase.java > be14afd > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java > 4a475ba > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbDDLAuditLog.java > 2cecdfd > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java > acb789f > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/AbstractTestWithStaticConfiguration.java > f8cc1d0 > > Diff: https://reviews.apache.org/r/28893/diff/ > > > Testing > ------- > > > Thanks, > > Arun Suresh > >
