----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50264/#review143015 -----------------------------------------------------------
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java (line 26) <https://reviews.apache.org/r/50264/#comment208684> Thank you for your nice work on adding more tests. For good practice, try not to import .*; sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java (line 36) <https://reviews.apache.org/r/50264/#comment208688> kerberos = true instead ? or you meant to have two test methods, one for non-kerberos and the other for kerberos. sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java (line 63) <https://reviews.apache.org/r/50264/#comment208685> Do we always use http not https for all metrics? sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java (line 80) <https://reviews.apache.org/r/50264/#comment208686> Not sure what is this empty method for? sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java (line 142) <https://reviews.apache.org/r/50264/#comment208687> To me isXXX natuarally returns TRUE or FALSE. Seems mp.isActive().getBooleanValue() also validate boolean results. Not sure why here return JsonNode obj. - Anne Yu On July 20, 2016, 9:24 p.m., Rahul Sharma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50264/ > ----------------------------------------------------------- > > (Updated July 20, 2016, 9:24 p.m.) > > > Review request for sentry, Anne Yu and Sravya Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > SENTRY-1331: Add a kerberos end to end test case to access isActive and isHa > metrics. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceMetrics.java > 3fff450c23d1ecd624d6fe406d5a7920b94fadd0 > > Diff: https://reviews.apache.org/r/50264/diff/ > > > Testing > ------- > > > Thanks, > > Rahul Sharma > >
