> On Feb. 20, 2019, 3:36 p.m., kalyan kumar kalvagadda wrote: > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java > > Lines 479-506 (patched) > > <https://reviews.apache.org/r/70013/diff/1/?file=2125866#file2125866line479> > > > > Functionally I'm good with the change but it is a code duplication as > > we will have same implementation in MetastoreAuthzBindingBase and > > HiveAuthzBindingHookBase. > > > > Could you move this caching to an abstract class and have these classes > > reuse it?
what is the abstract class would you suggest? HiveAuthzBindingHookBase extends AbstractSemanticAnalyzerHook, and MetastoreAuthzBindingBase extends MetaStorePreEventListener. They don't have comment parent. If I force them to derive from the same class, which extends AbstractSemanticAnalyzerHook and MetaStorePreEventListener, the inherantance structure would be very twisted just to share a common implementatioon of a function. I don't feel comfortable for that. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70013/#review212962 ----------------------------------------------------------- On Feb. 19, 2019, 9:59 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70013/ > ----------------------------------------------------------- > > (Updated Feb. 19, 2019, 9:59 p.m.) > > > Review request for sentry, Arjun Mishra, Haley Reeve, and kalyan kumar > kalvagadda. > > > Bugs: sentry-2501 > https://issues.apache.org/jira/browse/sentry-2501 > > > Repository: sentry > > > Description > ------- > > The filter in SentryMetaStoreFilterHook does not cache sentry privileges. > Therefore, for each item in the list, sentry client has to get privileges > from sentry server. > > To improve performance, we need to add cache in SentryMetaStoreFilterHook > > > Diffs > ----- > > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java > cdb6de4 > > sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetaStoreFilterHook.java > 312c5db > > > Diff: https://reviews.apache.org/r/70013/diff/1/ > > > Testing > ------- > > existing HMS tests succeeded > > > Thanks, > > Na Li > >
