> On May 2, 2016, 9:45 p.m., Gregory Chanan wrote: > > Why put the caching at the client layer instead of the provider backend > > layer? It seems unlikely to me that the average client actually wants > > caching or need caching, whereas an arbitrary external service probably > > does. > > Ashish Singh wrote: > Any arbitrary external service will need a client, right? Can't think of > a use case where a service would want to maintain a provider backend without > creating a client. Maybe I don't have complete context. Mind providing some > info here? > > Ashish Singh wrote: > Never mind, I completely mixed up Client impl and provider backend :(. > Yea, you are correct that it makes sense to have caching in provider backend > instead. Will update.
Ok, I think I am running into circles now :), but with each round hopefully there will be more info. So, provider backend is going to use a client as well, right? Depending on what the provider wants, it can enable or disable caching. I think it is better to have a generic client impl with caching support, which can then be used by multiple provider backends than having one of the provider backend have caching support. - Ashish ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46909/#review131381 ----------------------------------------------------------- On May 2, 2016, 9:32 p.m., Ashish Singh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46909/ > ----------------------------------------------------------- > > (Updated May 2, 2016, 9:32 p.m.) > > > Review request for sentry, Dapeng Sun, Gregory Chanan, Hao Hao, and Sravya > Tirukkovalur. > > > Bugs: SENTRY-1229 > https://issues.apache.org/jira/browse/SENTRY-1229 > > > Repository: sentry > > > Description > ------- > > SENTRY-1229: Add caching to SentryGenericServiceClientDefaultImpl. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java > 60502895a0fcdd781f5bd61b29a676a7c96f81b8 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java > dce3dade7f42fe35a849612c5caf2e98d2dac578 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 00e3fbde76ebf84704ee110adfca30869845a7b8 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > fcf0e7b9dc45eb12cc1b76f3084efd6340c039a4 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegrationWithCaching.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/46909/diff/ > > > Testing > ------- > > Extended unit tests. Will be running perf tests for Kafka-Sentry integration > with caching turned on. > > > Thanks, > > Ashish Singh > >
