Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-18 Thread Sergio Pena via Review Board 

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


Ship it!




Ship It!

- Sergio Pena


On Jan. 11, 2018, 10:14 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 11, 2018, 10:14 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee44892 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631fa 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e6953 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a31 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa629 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5fb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  501898bc 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
>  090999a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  edde886a 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
>  72d52de1 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
>  964a56cd 
> 
> 
> Diff: https://reviews.apache.org/r/65046/diff/4/
> 
> 
> Testing
> ---
> 
> 'mvn clean install' on testing cluster.
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-12 Thread kalyan kumar kalvagadda via Review Board 

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


Ship it!




Ship It!

- kalyan kumar kalvagadda


On Jan. 11, 2018, 10:14 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 11, 2018, 10:14 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee44892 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631fa 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e6953 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a31 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa629 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5fb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  501898bc 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
>  090999a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  edde886a 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
>  72d52de1 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
>  964a56cd 
> 
> 
> Diff: https://reviews.apache.org/r/65046/diff/4/
> 
> 
> Testing
> ---
> 
> 'mvn clean install' on testing cluster.
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-12 Thread kalyan kumar kalvagadda via Review Board 


> On Jan. 12, 2018, 8:51 p.m., kalyan kumar kalvagadda wrote:
> > Even file HiveNotificationFetcher.java and  
> > TestHiveNotificationFetcher.java should be moved. These file were not 
> > listed in the original ticket as these files were created after the ticked 
> > was opened.
> 
> Xinran Tinney wrote:
> Is there any other files other than these 2?

I was wrong, HiveNotificationFetcher.java and  TestHiveNotificationFetcher.java 
need not be moved.


- kalyan kumar


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


On Jan. 11, 2018, 10:14 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 11, 2018, 10:14 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee44892 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631fa 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e6953 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a31 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa629 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5fb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  501898bc 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
>  090999a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  edde886a 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
>  72d52de1 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
>  964a56cd 
> 
> 
> Diff: https://reviews.apache.org/r/65046/diff/4/
> 
> 
> Testing
> ---
> 
> 'mvn clean install' on testing cluster.
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-12 Thread kalyan kumar kalvagadda via Review Board 

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



Even file HiveNotificationFetcher.java and  TestHiveNotificationFetcher.java 
should be moved. These file were not listed in the original ticket as these 
files were created after the ticked was opened.

- kalyan kumar kalvagadda


On Jan. 11, 2018, 10:14 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 11, 2018, 10:14 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee44892 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631fa 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e6953 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a31 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa629 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5fb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  501898bc 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
>  090999a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  edde886a 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
>  72d52de1 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
>  964a56cd 
> 
> 
> Diff: https://reviews.apache.org/r/65046/diff/4/
> 
> 
> Testing
> ---
> 
> 'mvn clean install' on testing cluster.
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-11 Thread Xinran Tinney 

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

(Updated Jan. 11, 2018, 10:14 p.m.)


Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
---

Added all the actual imports instead of using .*;


Repository: sentry


Description
---

We have several important classes - e.g. HMSFollower, NotificationProcessor, 
CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which is 
weird - they should be in provider.db.service.persistent instead.


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 eee44892 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 6c4631fa 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
 558e6953 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
 86ff47e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
 aa1b6a31 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
 097aa629 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
 77634cf2 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
 8e80d558 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
 d09da5fb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 7e774b4a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
 43535a7b 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
 501898bc 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
 090999a0 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
 edde886a 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
 72d52de1 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
 964a56cd 


Diff: https://reviews.apache.org/r/65046/diff/4/

Changes: https://reviews.apache.org/r/65046/diff/3-4/


Testing
---

'mvn clean install' on testing cluster.


Thanks,

Xinran Tinney

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-10 Thread Sergio Pena via Review Board 

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




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
Line 35 (original), 35 (patched)


Instead of using a wildcard, can you write each import one at a time?


- Sergio Pena


On Jan. 9, 2018, 9:45 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 9, 2018, 9:45 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee44892 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631fa 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e6953 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e0 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a31 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa629 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf2 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d558 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5fb 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4a 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7b 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
>  501898bc 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
>  090999a0 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
>  edde886a 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
>  72d52de1 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
>  964a56cd 
> 
> 
> Diff: https://reviews.apache.org/r/65046/diff/3/
> 
> 
> Testing
> ---
> 
> 'mvn clean install' on testing cluster.
> 
> 
> Thanks,
> 
> Xinran Tinney
> 
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-09 Thread Xinran Tinney 

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

(Updated Jan. 9, 2018, 9:45 p.m.)


Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Changes
---

Sync with SENTRY-2085


Repository: sentry


Description
---

We have several important classes - e.g. HMSFollower, NotificationProcessor, 
CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which is 
weird - they should be in provider.db.service.persistent instead.


Diffs (updated)
-

  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 eee44892 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 6c4631fa 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
 558e6953 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
 86ff47e0 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
 aa1b6a31 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
 097aa629 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
 77634cf2 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
 8e80d558 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
 d09da5fb 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 7e774b4a 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
 43535a7b 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
 501898bc 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestCounterWait.java
 090999a0 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestHMSFollower.java
 edde886a 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestLeaderStatusMonitor.java
 72d52de1 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestNotificationProcessor.java
 964a56cd 


Diff: https://reviews.apache.org/r/65046/diff/3/

Changes: https://reviews.apache.org/r/65046/diff/2-3/


Testing
---

'mvn clean install' on testing cluster.


Thanks,

Xinran Tinney

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-09 Thread Na Li via Review Board 

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




sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
Line 394 (original), 394 (patched)


why do you remove the part to throw exception?



sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
Line 837 (original), 827 (patched)


It seems you are reverting changes made in SENTRY-2085. Can you sync the 
code, rebase your commit and then update the review?


- Na Li


On Jan. 9, 2018, 5:32 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 9, 2018, 5:32 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  7565a34 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
>  447deaf 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
>  3bbf6fb 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java
>  4092fe4 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
>  8d28ccc 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java
>  6344435 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee4489 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  73fcda8 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java
>  9048d76 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java
>  00b5cf6 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f3 
>   
> sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java
>  c8f2bed 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  2fbad36 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e695 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa62 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d55 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7 
>   
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
>  cc72b33 
>   
>

Re: Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-09 Thread Steve Moist via Review Board 

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




sentry-dist/src/license/THIRD-PARTY.properties
Line 1 (original), 1 (patched)


Remove this file.  This should not be checked in.


- Steve Moist


On Jan. 9, 2018, 3:44 p.m., Xinran Tinney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65046/
> ---
> 
> (Updated Jan. 9, 2018, 3:44 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
> hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> We have several important classes - e.g. HMSFollower, NotificationProcessor, 
> CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which 
> is weird - they should be in provider.db.service.persistent instead.
> 
> 
> Diffs
> -
> 
>   
> sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
>  7565a34 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
>  447deaf 
>   
> sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
>  3bbf6fb 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java
>  4092fe4 
>   
> sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
>  5c2a301 
>   
> sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
>  8d28ccc 
>   
> sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java
>  6344435 
>   sentry-dist/src/license/THIRD-PARTY.properties 22429fc 
>   
> sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
>  eee4489 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
>  73fcda8 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java
>  9048d76 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java
>  00b5cf6 
>   
> sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
>  a9b98f3 
>   
> sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java
>  c8f2bed 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CounterWait.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LeaderStatusMonitor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
>  PRE-CREATION 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  6c4631f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
>  2fbad36 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
>  558e695 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
>  86ff47e 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
>  aa1b6a3 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
>  097aa62 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
>  77634cf 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
>  8e80d55 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
>  d09da5f 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
>  7e774b4 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  43535a7 
>   
>

Review Request 65046: SENTRY-1819 HMSFollower and friends do not belong in sentry.service.thrift

2018-01-09 Thread Xinran Tinney 

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

Review request for sentry, Alexander Kolbasov, Arjun Mishra, Colm O 
hEigeartaigh, kalyan kumar kalvagadda, Na Li, and Sergio Pena.


Repository: sentry


Description
---

We have several important classes - e.g. HMSFollower, NotificationProcessor, 
CounterWait, LeaderStatusMonitor in the sentry.service.thrift package which is 
weird - they should be in provider.db.service.persistent instead.


Diffs
-

  
sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
 7565a34 
  
sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
 447deaf 
  
sentry-binding/sentry-binding-hive/src/test/java/org/apache/sentry/binding/hive/TestHiveAuthzBindings.java
 3bbf6fb 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SentrySolrPluginImpl.java
 4092fe4 
  
sentry-binding/sentry-binding-solr/src/main/java/org/apache/sentry/binding/solr/authz/SolrAuthzBinding.java
 5c2a301 
  
sentry-binding/sentry-binding-solr/src/test/java/org/apache/sentry/binding/solr/TestSolrAuthzBinding.java
 8d28ccc 
  
sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/exception/SentryGroupNotFoundException.java
 6344435 
  sentry-dist/src/license/THIRD-PARTY.properties 22429fc 
  
sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java
 eee4489 
  
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/AuthorizationProvider.java
 73fcda8 
  
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/GroupMappingService.java
 9048d76 
  
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/HadoopGroupMappingService.java
 00b5cf6 
  
sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
 a9b98f3 
  
sentry-provider/sentry-provider-common/src/test/java/org/apache/sentry/provider/common/TestNoAuthorizationProvider.java
 c8f2bed 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/CounterWait.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HMSFollower.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/LeaderStatusMonitor.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 6c4631f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
 2fbad36 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/CounterWait.java
 558e695 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSClient.java
 86ff47e 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java
 aa1b6a3 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveNotificationFetcher.java
 097aa62 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HiveSimpleConnectionFactory.java
 77634cf 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/LeaderStatusMonitor.java
 8e80d55 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/NotificationProcessor.java
 d09da5f 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryHMSClient.java
 7e774b4 
  
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryService.java
 43535a7 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java
 cc72b33 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestCounterWait.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java
 PRE-CREATION 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollowerSentryStoreIntegration.java
 501898b 
  
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestLeaderStatusMonitor.java
 PRE-CREATION