----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21468/#review43212 -----------------------------------------------------------
Ship it! sentry-provider/sentry-provider-cache/pom.xml <https://reviews.apache.org/r/21468/#comment77294> why does this need a dependency on shiro? sentry-provider/sentry-provider-cache/pom.xml <https://reviews.apache.org/r/21468/#comment77292> Why does the provider have a dependency on model-db? sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/PrivilegeCache.java <https://reviews.apache.org/r/21468/#comment77331> It seems the only use case currently for close is to support your test case. Instead of baking this into the interface, how about just making that part of the test cleanup? In any event this needs a comment on what it should do and a close() without an "open" don't make sense. sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimpleCacheProviderBackend.java <https://reviews.apache.org/r/21468/#comment77295> nit: assert initialized()? Or even better, throw an exception here explaining that the binding handle was null so the caller knows what to fix. sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackendContext.java <https://reviews.apache.org/r/21468/#comment77332> Comment on public function. sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackendContext.java <https://reviews.apache.org/r/21468/#comment77333> Instead of an "Object" should the bindingHandle be a ProviderBackend? - Lenni Kuff On May 14, 2014, 11:29 p.m., Prasad Mujumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21468/ > ----------------------------------------------------------- > > (Updated May 14, 2014, 11:29 p.m.) > > > Review request for sentry, Lenni Kuff and Sravya Tirukkovalur. > > > Bugs: SENTRY-156 > https://issues.apache.org/jira/browse/SENTRY-156 > > > Repository: sentry > > > Description > ------- > > The patch implements a simple cache based policy provider that reads the > privilege metadata from a cache interface. > > > Diffs > ----- > > sentry-provider/pom.xml 9bec058 > sentry-provider/sentry-provider-cache/pom.xml PRE-CREATION > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/PrivilegeCache.java > PRE-CREATION > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimpleCacheProviderBackend.java > PRE-CREATION > > sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/PrivilegeCacheTestImpl.java > PRE-CREATION > > sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestCacheProvider.java > PRE-CREATION > > sentry-provider/sentry-provider-cache/src/test/resources/test-authz-provider-local-group-mapping.ini > PRE-CREATION > > sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackendContext.java > f45d23d > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java > 22f1b08 > > Diff: https://reviews.apache.org/r/21468/diff/ > > > Testing > ------- > > Added new unit test cases for the provider. > > > Thanks, > > Prasad Mujumdar > >
