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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 95 (original)
<https://reviews.apache.org/r/58221/#comment246936>

    I need to revert it back. The code should be in constructor here, not in 
the constructor used for testing.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Lines 132 (patched)
<https://reviews.apache.org/r/58221/#comment246937>

    I will remove it



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Line 171 (original), 171 (patched)
<https://reviews.apache.org/r/58221/#comment246938>

    I will move it to start service



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 313 (patched)
<https://reviews.apache.org/r/58221/#comment246940>

    I will check both



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 314 (patched)
<https://reviews.apache.org/r/58221/#comment246941>

    will update



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 318 (patched)
<https://reviews.apache.org/r/58221/#comment246942>

    it does not hurt to check here.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 322 (patched)
<https://reviews.apache.org/r/58221/#comment246943>

    it would be easier to read the code by keeping this section outside of 
previous finally block



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 326 (patched)
<https://reviews.apache.org/r/58221/#comment246944>

    will move



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 331 (patched)
<https://reviews.apache.org/r/58221/#comment246945>

    will add



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 332 (patched)
<https://reviews.apache.org/r/58221/#comment246946>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 363 (patched)
<https://reviews.apache.org/r/58221/#comment246947>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
Lines 368 (patched)
<https://reviews.apache.org/r/58221/#comment246948>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 24 (patched)
<https://reviews.apache.org/r/58221/#comment246950>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 157 (patched)
<https://reviews.apache.org/r/58221/#comment246951>

    Will fix



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 165 (patched)
<https://reviews.apache.org/r/58221/#comment246952>

    Other functions in this class are public. Why do we want to limit it to 
just package private?



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 166 (patched)
<https://reviews.apache.org/r/58221/#comment246953>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 177 (patched)
<https://reviews.apache.org/r/58221/#comment246954>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
Lines 180 (patched)
<https://reviews.apache.org/r/58221/#comment246955>

    will change



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
Line 172 (original), 172 (patched)
<https://reviews.apache.org/r/58221/#comment246957>

    I believe it can be zero if it does not create local meta store.



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Line 421 (original), 424 (patched)
<https://reviews.apache.org/r/58221/#comment246958>

    I want to avoid two tests interfere with each other if they are running in 
parallel. I was trying to check what caused the test failure



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
Line 441 (original), 444 (patched)
<https://reviews.apache.org/r/58221/#comment246959>

    will do


- Na Li


On May 3, 2017, 7:20 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58221/
> -----------------------------------------------------------
> 
> (Updated May 3, 2017, 7:20 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, kalyan kumar 
> kalvagadda, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> SENTRY-1649 move HMS follower to runServer
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  ec8676e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  132db63 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryServiceUtil.java
>  ce73358 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java
>  834ed41 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationEnd2End.java
>  1ace07c 
> 
> 
> Diff: https://reviews.apache.org/r/58221/diff/23/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to