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




sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java
Lines 92 (patched)
<https://reviews.apache.org/r/59174/#comment248263>

    Need a space between methods.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java
Lines 102 (patched)
<https://reviews.apache.org/r/59174/#comment248264>

    Need a space between methods.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java
Lines 112 (patched)
<https://reviews.apache.org/r/59174/#comment248265>

    Optional:
    
    I've seen this line duplicated many times when creating events for testing. 
Should it be good to have an utility method that creates these events? 
Something like: 
    
    newDatabaseEvent(ID, new Database());
    
    It may be in a common place for utilities classes for testing.



sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java
Lines 134-135 (patched)
<https://reviews.apache.org/r/59174/#comment248269>

    Are we testing the situation when two HMSFollower are processing 
notifications at the same time? Do we allow this situation?


- Sergio Pena


On May 11, 2017, 12:51 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59174/
> -----------------------------------------------------------
> 
> (Updated May 11, 2017, 12:51 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, 
> Sergio Pena, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Bugs: SENTRY-1761
>     https://issues.apache.org/jira/browse/SENTRY-1761
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Added test to make sure that SENTRY_HMS_NOTIFICATION_ID table is properly 
> updated even when multiple HMSFollowers are processing notifications from HMS.
> 
> 
> Diffs
> -----
> 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestConcurrentHMSFollowers.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59174/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>

Reply via email to