----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58412/#review172007 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2519 (patched) <https://reviews.apache.org/r/58412/#comment245047> Is it possible some paths should be removed from existing MAuthzPathsMapping entry? For example, user wants to revoke the privilege of some tables from a role. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2535 (patched) <https://reviews.apache.org/r/58412/#comment245048> could the exception type to be more specific? Such as noSuchPath or its parent exception? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2562 (patched) <https://reviews.apache.org/r/58412/#comment245063> should we continue delete the path, and then throw the exceptions? Those changes work in Hive, so Sentry needs to be more tolerate to errors. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2579 (patched) <https://reviews.apache.org/r/58412/#comment245049> If deleting one entry fails, should the operation continues for the rest of entries before throwing exception, or throw exception right away? The changes are from Hive, and they succeeds at Hive. If exception happens at Sentry, should we be more tolenrate on the error? sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Lines 2646 (patched) <https://reviews.apache.org/r/58412/#comment245062> what is the sequence when renaming happens? Will the old path renamed to new path, and then rename authz path mapping? If that is the case, then "mOldPath == null" should be allowed. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 2468 (original), 2649 (patched) <https://reviews.apache.org/r/58412/#comment245061> should this be "mAuthzPathsMapping != null"? You call mAuthzPathsMapping.removePath(mOldPath) afterwards. sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 2468 (original), 2649 (patched) <https://reviews.apache.org/r/58412/#comment245064> should this be mAuthzPathsMapping != null sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java Line 2473 (original), 2661 (patched) <https://reviews.apache.org/r/58412/#comment245066> should we throw exception for mOldPath == null? Is that possible that old path is renamed, and then this function is called? - Na Li On April 13, 2017, 6:58 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58412/ > ----------------------------------------------------------- > > (Updated April 13, 2017, 6:58 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > [WIP]SENTRY-1587: Refactor SentryStore transaction to persist a single path > transcation bundled with corresponding delta path change > > Change-Id: Ice77f631e92c0e74b42ae644e1b2f90c6089e62f > > > Diffs > ----- > > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/FullUpdateInitializer.java > 90aaaef0e15306627d7108f12a74a29848055c0b > > sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/PathsUpdate.java > 14e967aa1065f16e8d4c3f61db2f9055959fa9e6 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MAuthzPathsMapping.java > c22364fa26c80415827313f4d26f6c53b71b6f6c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > aaea9790282f9136302eec64107cc86391a4d6ff > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 16676fb13b0d5015aefe892a6f7e46812ea75124 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 31a309bdc789bd7a997f7654e30f2021ecb5b616 > > > Diff: https://reviews.apache.org/r/58412/diff/1/ > > > Testing > ------- > > > Thanks, > > Hao Hao > >
