> On May 26, 2016, 10:58 p.m., Lenni Kuff wrote: > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimplePrivilegeCache.java, > > line 86 > > <https://reviews.apache.org/r/47872/diff/4/?file=1395744#file1395744line86> > > > > Should we always store the auth key as lowercase to avoid copies ? > > Hao Hao wrote: > I think it is stored as lowercase now?
Right now you call getKey().toLowerCase() which will copy the key and convert it to lower case. Question was whether it is better to store the lowercase value in getKey() (making it clear that keys are case insensitive/always lowercase). This would avoid extra copies. > On May 26, 2016, 10:58 p.m., Lenni Kuff wrote: > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java, > > line 117 > > <https://reviews.apache.org/r/47872/diff/4/?file=1395737#file1395737line117> > > > > Would it be clearner to have an AuthorizableHierarchy type that > > internally stores a list of key/values? > > > > You could then have an interface like: > > > > authzHierarchy.toKeyValues(); > > authzHierarchy.toAuthzString(); > > Hao Hao wrote: > I think the key/values is not only for AuthorizableHierarchy, it also > contains ACTION information. Not sure if we should wrap it as > AuthorizableHierarchy? Maybe AuthorizableHierarchy isn't the right thing, but seems like we should have some abstraction on top of a List<KeyValue> objects. - Lenni ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47872/#review135080 ----------------------------------------------------------- On May 27, 2016, 1:29 a.m., Hao Hao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47872/ > ----------------------------------------------------------- > > (Updated May 27, 2016, 1:29 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > SENTRY-1291: SimpleCacheProviderBackend.getPrivileges should return the > permission based on authorizationhierarchy > > Change-Id: Ifa38aee17c232c50cd6b992126837a756f3aaa7f > > Changed SimpleCacheProviderBackend.getPrivileges to return the permissions > based on authorizationhierarchy. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/privilege/hive/TestCommonPrivilegeForHive.java > c719802150a6dc5e4331af1cdcddabe0329554e9 > > sentry-binding/sentry-binding-kafka/src/test/java/org/apache/sentry/privilege/kafka/TestKafkaWildcardPrivilege.java > a616f67b7eeb3b106944137745bb215c04622a93 > > sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/privilege/solr/TestCommonPrivilegeForSearch.java > 294091cba0a8989d327de24d50aa81dc3ce238f9 > > sentry-binding/sentry-binding-sqoop/src/test/java/org/apache/sentry/privilege/sqoop/TestCommonPrivilegeForSqoop.java > 92e929080fe6fec908141f98d546ef481946a88b > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java > dedd90886e9450054b2cc0a796677318add9b3c0 > > sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/Privilege.java > e9f46094cf79a3f91c953c9b3bbecd3aecc2ebad > > sentry-policy/sentry-policy-engine/src/main/java/org/apache/sentry/policy/engine/common/CommonPolicyEngine.java > ee25427bd47834fb94915f7df5d5a0dca850c9bb > > sentry-policy/sentry-policy-indexer/src/main/java/org/apache/sentry/policy/indexer/IndexerWildcardPrivilege.java > 71d2a665dc36cc853cd0a6efef0b654ac044e28f > > sentry-policy/sentry-policy-indexer/src/test/java/org/apache/sentry/policy/indexer/TestCommonPrivilegeForIndexer.java > 2a3bde72aee7eb53150c5524313da18ff201dd7e > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/PrivilegeCache.java > 28c5b769d4d77c0f270833d5856f7ee582a950db > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimpleCacheProviderBackend.java > b3db752da85a71d8c94f6def7ad0aac600545bd8 > > sentry-provider/sentry-provider-cache/src/main/java/org/apache/sentry/provider/cache/SimplePrivilegeCache.java > dd0c39ab79470e2477716c5ec4e5aed1d4ec0f55 > > sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/PrivilegeCacheTestImpl.java > 79e047ee8037c9463fd812bd57dbad045952a36b > > sentry-provider/sentry-provider-cache/src/test/java/org/apache/sentry/provider/cache/TestSimplePrivilegeCache.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java > 1d1da5e41afb8b32e9aded4b9f06bd285f221a61 > > Diff: https://reviews.apache.org/r/47872/diff/ > > > Testing > ------- > > All existing hive e2e tests. > > > Thanks, > > Hao Hao > >
