> On April 5, 2016, 6:33 a.m., Jerry Chen wrote: > > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java, > > line 296 > > <https://reviews.apache.org/r/45730/diff/2/?file=1325652#file1325652line296> > > > > It would be better to use null to indicate there is no filter. So the > > exportPolicy would better to handle null and empty filter the same way.
done, also add test case for null and empty. > On April 5, 2016, 6:33 a.m., Jerry Chen wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java, > > line 910 > > <https://reviews.apache.org/r/45730/diff/2/?file=1325655#file1325655line910> > > > > Comments here: > > 1. dbName: try to use full name if it is not too long: databaseName. > > 2. Prefer to use null for indication of a parameter has no value. > > 3. Does Sentry has static utility method for parse this? try use a > > static method. If there is not, contribute this as one instead embeded into > > the portion of code. Thanks for the comments, all done. - Colin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45730/#review127041 ----------------------------------------------------------- On April 5, 2016, 5:31 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45730/ > ----------------------------------------------------------- > > (Updated April 5, 2016, 5:31 a.m.) > > > Review request for sentry. > > > Repository: sentry > > > Description > ------- > > Update Sentry Policy Service for export with specific auth object > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/SentryConfigTool.java > 73b0941 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java > de50adb > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java > edc5661 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java > 8881d82 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceImportExport.java > dbe4a27 > > Diff: https://reviews.apache.org/r/45730/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
