> On June 6, 2018, 6:57 p.m., kalyan kumar kalvagadda wrote: > > sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java > > Lines 43 (patched) > > <https://reviews.apache.org/r/67447/diff/2/?file=2036102#file2036102line43> > > > > WHy do you need to use a mock here? > > > > SentryStore has API's > > alterSentryUserGrantPrivileges > > alterSentryUserRevokePrivileges > > > > You could use sentrustore API's to grant privileges t users and use the > > client API to retrieve the privileges. > > Sergio Pena wrote: > I'm testing only the logic of the Sentry client. I like to use mocks when > testing specific things and not the whole functionality from the client to > the server and persisting on the DB. This makes tests slower the more and > more we add.
I agree. This is functional test, and we should focus on specific class to help isolate the issue. The only thing to be careful is the mock behavior should be the same as real SentryStore. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67447/#review204411 ----------------------------------------------------------- On June 6, 2018, 5:22 p.m., Sergio Pena wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67447/ > ----------------------------------------------------------- > > (Updated June 6, 2018, 5:22 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda and Na Li. > > > Bugs: SENTRY-2162 > https://issues.apache.org/jira/browse/SENTRY-2162 > > > Repository: sentry > > > Description > ------- > > This patch implements the API to list user privileges and provide > authorization using user privileges. > It also allows Hive to use the SHOW GRANT USER command when listing the user > privileges. > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/DefaultSentryAccessController.java > fc2427cbf2289f79550c25298ad1f76ac56ae7b4 > sentry-service/sentry-service-api/pom.xml > ba7a7ced332ce9b8592c5ec53f9961f26c4c8f10 > > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClient.java > 4ba7e801156f38fa7e2eecb03316c2a096c55248 > > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java > c964318d93cb7b8ea2724852c871ad5ec5fcfc06 > > sentry-service/sentry-service-api/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyServiceClientDefaultImpl.java > PRE-CREATION > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryMetrics.java > 80a634392bf6e9d9d12019bc50e3e4136a9605ff > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyStoreProcessor.java > b5ef200c72e48d5d288c167403f1abdd6bca9ce7 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/MockGroupMappingService.java > PRE-CREATION > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/api/service/thrift/TestSentryPolicyStoreProcessor.java > b028303d42f4e3a71f65455178df52da07fe6c20 > > > Diff: https://reviews.apache.org/r/67447/diff/2/ > > > Testing > ------- > > > Thanks, > > Sergio Pena > >
