> On May 31, 2017, 7:27 p.m., Sergio Pena wrote:
> > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
> > Lines 134 (patched)
> > <https://reviews.apache.org/r/59350/diff/1/?file=1723426#file1723426line134>
> >
> >     The method name does not match with what it is return. Some people may 
> > see newSentryStore() as a method that creates a new SentryStore every time 
> > is called, but this creates it only once. Should we rename the method? 
> > Perhaps have a singleton class?
> 
> Alexander Kolbasov wrote:
>     How about renaming it to getSentryStore()
> 
> Sergio Pena wrote:
>     That looks good. What about SentryStore.getInstance() ? I've seen this 
> name on singleton examples.

getInstance() looks good. Will change the name.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59350/#review176501
-----------------------------------------------------------


On May 17, 2017, 7:19 p.m., Alexander Kolbasov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59350/
> -----------------------------------------------------------
> 
> (Updated May 17, 2017, 7:19 p.m.)
> 
> 
> Review request for sentry, Brian Towles, Hao Hao, kalyan kumar kalvagadda, Na 
> Li, Sergio Pena, and Vamsee Yarlagadda.
> 
> 
> Bugs: SENTRY-1743
>     https://issues.apache.org/jira/browse/SENTRY-1743
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1743: Two SentrySTore instances are one too many
> 
> 
> Diffs
> -----
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java
>  3be6a0bf38d03f81df4aae3a878d1e6ecae52be7 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  f0d2a6b3a74d8d88976aeec32bba901d02e8329f 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryAdminServlet.java
>  8a8bbd35c1e327fb8b613311daaafef46561b2bb 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  e9d9eec3526f2894199e81a253709dc3ed380b5b 
>   
> sentry-tests/sentry-tests-sqoop/src/test/java/org/apache/sentry/tests/e2e/sqoop/AbstractSqoopSentryTestBase.java
>  67de5acb36797b23261e5023c308222ad3344ea7 
> 
> 
> Diff: https://reviews.apache.org/r/59350/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>

Reply via email to