Re: Review Request 54808: SENTRY-1526 Sentry processed stays alive after being killed

2016-12-15 Thread Misha Dmitriev

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


Ship it!




Ship It!

- Misha Dmitriev


On Dec. 16, 2016, 7:23 a.m., Alexander Kolbasov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54808/
> ---
> 
> (Updated Dec. 16, 2016, 7:23 a.m.)
> 
> 
> Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Vamsee 
> Yarlagadda, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> SENTRY-1526 Sentry processed stays alive after being killed
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java
>  578364933a3cdcf6c142b836360a83d322fe5c11 
> 
> Diff: https://reviews.apache.org/r/54808/diff/
> 
> 
> Testing
> ---
> 
> Now process successfully dies after hitting ^C.
> 
> 
> Thanks,
> 
> Alexander Kolbasov
> 
>



Review Request 54808: SENTRY-1526 Sentry processed stays alive after being killed

2016-12-15 Thread Alexander Kolbasov

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

Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Vamsee Yarlagadda, 
and Vadim Spector.


Repository: sentry


Description
---

SENTRY-1526 Sentry processed stays alive after being killed


Diffs
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/service/thrift/SentryService.java
 578364933a3cdcf6c142b836360a83d322fe5c11 

Diff: https://reviews.apache.org/r/54808/diff/


Testing
---

Now process successfully dies after hitting ^C.


Thanks,

Alexander Kolbasov



Review Request 54798: SENTRY-1546 Generic Policy provides bad error messages for Sentry exceptions

2016-12-15 Thread kalyan kumar kalvagadda

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

Review request for sentry, Alexander Kolbasov, Hao Hao, Vamsee Yarlagadda, and 
Vadim Spector.


Repository: sentry


Description
---

Added code changes to SentryStore to have proper message (where the exceptions 
are originated). If this is done we can avoid decorating them later.
I saw issue with exceptions created while role create/drop and made required 
changes.


Diffs
-

  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
 742798d 
  
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
 898632d 

Diff: https://reviews.apache.org/r/54798/diff/


Testing
---

I have performed tests using Sentrytool Client.


Thanks,

kalyan kumar kalvagadda



Re: Review Request 54525: SENTRY-1541: toSentryPrivilege() should not copy fields that are not set in the source

2016-12-15 Thread Vamsee Yarlagadda


> On Dec. 13, 2016, 6:28 p.m., Alexander Kolbasov wrote:
> > 1) Do we have any other users of fromNullCol or it should be removed as 
> > well?
> > 2) This relates to SENTRY-1540 since now isMultiActionsSupported() becomes 
> > a real code and this changes the existing semantics some. Is it Ok or not?

bq. 1) Do we have any other users of fromNullCol or it should be removed as 
well?
I verified that there are no users of fromNullCol and that's why it was removed 
in the last patch.

bq. 2) This relates to SENTRY-1540 since now isMultiActionsSupported() becomes 
a real code and this changes the existing semantics some. Is it Ok or not?
As part of this change we are only ensuring thrift objects has NULL attributes 
for the fields that are not set. And this still preserves the functionality of 
isMultiActionsSupported as they are only checking for NULL variables in thrift 
objects.


- Vamsee


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


On Dec. 12, 2016, 7:57 p.m., Vamsee Yarlagadda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54525/
> ---
> 
> (Updated Dec. 12, 2016, 7:57 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar 
> kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> Only sets the fields in TSentryPrivilege that are set in TSentryAuthorizable
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  e8d0e2505c5b178cae3d27e7d85caa652f630a2d 
> 
> Diff: https://reviews.apache.org/r/54525/diff/
> 
> 
> Testing
> ---
> 
> ```bash
> vamsee-MBP:sentry-service-server vamsee$ mvn clean test 
> -Dtest=TestSentryStore -DfailIfNoTests=false 
> ...
> ---
>  T E S T S
> ---
> Running org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 2016-12-08 00:08:59.772 java[5207:544535] Unable to load realm info from 
> SCDynamicStore
> Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 18.302 sec - 
> in org.apache.sentry.provider.db.service.persistent.TestSentryStore
> 
> Results :
> 
> Tests run: 41, Failures: 0, Errors: 0, Skipped: 2
> 
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 44.421s
> [INFO] Finished at: Thu Dec 08 00:09:18 PST 2016
> [INFO] Final Memory: 67M/687M
> [INFO] 
> 
> ```
> 
> 
> Thanks,
> 
> Vamsee Yarlagadda
> 
>