> On 十二月 2, 2014, 11:57 p.m., Arun Suresh wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/genericModel/service/thrift/NotificationHandler.java, > > line 22 > > <https://reviews.apache.org/r/26131/diff/1/?file=708103#file708103line22> > > > > Do you think it is possible to reuse the Existing NotificationHandler ?
The generic model use a new thrift API, becuase it doesn't want to affect the origin functionality. The arguments for the origin notificationHandler is different, so it has some diffculties to reuse the exsiting source code. > On 十二月 2, 2014, 11:57 p.m., Arun Suresh wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/genericModel/service/thrift/NotificationHandlerInvoker.java, > > line 2 > > <https://reviews.apache.org/r/26131/diff/1/?file=708104#file708104line2> > > > > Do you think it is possible to reuse the Existing > > NotificationHandlerInvoker ? The generic model use a new thrift API, becuase it doesn't want to affect the origin functionality. The arguments for the origin notificationHandler is different, so it has some diffculties to reuse the exsiting source code. > On 十二月 2, 2014, 11:57 p.m., Arun Suresh wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/genericModel/service/thrift/SentryGenericServiceClient.java, > > line 73 > > <https://reviews.apache.org/r/26131/diff/1/?file=708107#file708107line73> > > > > Maybe refactor this... so that it can be shared with > > SentryHDFSServiceClient and SentryPolicyServiceCLient.. Hi, Arun. Thanks for your comment. I think you give me a good direction about refactor the patch. Moving all operations that involve just role or group to a separate class, the operations related the generic privileges to another class. I think it is a brilliant idea. But I recommend that the main part of generic model should be refactor is the generic sentry store(SENTRY-405). This SENTRY-404 patch hasn't a lot of code repetition, it just receive the request and redirect to the sentry store for processing.In that SENTRY-405 patch, I accept your advice, let the operations that involve role or group parts to reuse the existing code, then write a new privilege operator to process the other requests. I have already update that pacth, please help me review it and feel free give your comments. Thanks a lot - shen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26131/#review63607 ----------------------------------------------------------- On 十二月 5, 2014, 2:18 p.m., shen guoquan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26131/ > ----------------------------------------------------------- > > (Updated 十二月 5, 2014, 2:18 p.m.) > > > Review request for sentry, Xiaomeng Huang, Arun Suresh, Colin Ma, Dapeng Sun, > Prasad Mujumdar, and Sravya Tirukkovalur. > > > Bugs: sentry-404 > https://issues.apache.org/jira/browse/sentry-404 > > > Repository: sentry > > > Description > ------- > > The previous thrift interface is not general and mainly fit for database > authorization model,like hive/impala. If the sentry want to binding the other > no-database authorization model such as Solr/ElasticSearch, the current > sentry service can't handle this requirement. So there is a requirement to > extend the sentry thrift interface to adapt the generic authorization model > and add a new processor to handle thrift requests. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/NotificationHandler.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/NotificationHandlerInvoker.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessorFactory.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/PolicyStoreConstants.java > 34bec93 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java > 06aaacd > > sentry-provider/sentry-provider-db/src/main/resources/sentry_common_service.thrift > 9456274 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_generic_policy_service.thrift > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java > PRE-CREATION > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericServiceIntegration.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/26131/diff/ > > > Testing > ------- > > > Thanks, > > shen guoquan > >
