> On July 22, 2014, 8:18 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > lines 1115-1116 > > <https://reviews.apache.org/r/23600/diff/1/?file=634111#file634111line1115> > > > > curious, why you had to change this? Not part of this patch but also > > the scope should be lower cased right? > > Arun Suresh wrote: > there was a testcase that was setting uppercase Actions... Did not want > to change the test case, also, I thought doing this here will enforce > lowercase. our actions are supposed to be lowercase rite ?
Yes, I believe we do lower case everything except for usernames and groupnames. > On July 22, 2014, 8:18 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql, > > line 82 > > <https://reviews.apache.org/r/23600/diff/1/?file=634113#file634113line82> > > > > Wonder why are some in quotes? Are they system keywords? > > http://www-01.ibm.com/support/docview.wss?uid=swg21164500 > > Arun Suresh wrote: > looks like.. these columns had "" where they were declared.. so decided > to copy as it is.. Ok, might be best to do a script test on all dbs. Can you make sure these changes work? > On July 22, 2014, 8:18 p.m., Sravya Tirukkovalur wrote: > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift, > > line 36 > > <https://reviews.apache.org/r/23600/diff/1/?file=634118#file634118line36> > > > > We should probably not provide a default for serverName? We should make > > serverName strictly required. > > Arun Suresh wrote: > so looks like there is something funny going on in our thrift version.. I > am able to create a TSentryPrivilege with null serverName. This seemed to > work for me finally Hmm, that does not seem right. We should enforce that serverName can never be null. Can you add a test case for it if possible? - Sravya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23600/#review48400 ----------------------------------------------------------- On July 24, 2014, 12:32 a.m., Arun Suresh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23600/ > ----------------------------------------------------------- > > (Updated July 24, 2014, 12:32 a.m.) > > > Review request for sentry, Jarek Cecho, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-339 : > > Remove the need to construct a privilegeName to check for uniqueness. > - Remove the PrivilegeName column > - Remove the constructPrivilegeName() method > - Updated the sql scripts > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java > 9e8ac4c > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/MSentryPrivilege.java > f8491db > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/model/package.jdo > 945227e > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > ff8acdc > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 3bb7285 > sentry-provider/sentry-provider-db/src/main/resources/sentry-db2-1.4.0.sql > f2a62d2 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-derby-1.4.0.sql > f2a62d2 > > sentry-provider/sentry-provider-db/src/main/resources/sentry-mysql-1.4.0.sql > 70f4dbb > > sentry-provider/sentry-provider-db/src/main/resources/sentry-oracle-1.4.0.sql > 363590e > > sentry-provider/sentry-provider-db/src/main/resources/sentry-postgres-1.4.0.sql > 5dfae03 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > fdc7b9c > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 7637376 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServerWithoutKerberos.java > 79579c6 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java > a4ae291 > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPolicyImport.java > 948b0c4 > > Diff: https://reviews.apache.org/r/23600/diff/ > > > Testing > ------- > > Ran all e2e tests and unit tests. > > > Thanks, > > Arun Suresh > >
