Re: Review Request 67555: SENTRY-2267: Listing user privileges fails because roleName field is required on Thrift

2018-06-12 Thread Na Li via Review Board 

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


Ship it!




Ship It!

- Na Li


On June 12, 2018, 6:46 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67555/
> ---
> 
> (Updated June 12, 2018, 6:46 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2267
> https://issues.apache.org/jira/browse/sentry-2267
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Sentry client was failing to get a list of user privileges because the 
> roleName is required.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  f0f08ea3d 
> 
> 
> Diff: https://reviews.apache.org/r/67555/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 67555: SENTRY-2267: Listing user privileges fails because roleName field is required on Thrift

2018-06-12 Thread Arjun Mishra via Review Board 


> On June 12, 2018, 6:58 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
> > Lines 270 (patched)
> > 
> >
> > Is this just for the dropRole() method?
> 
> Sergio Pena wrote:
> It's not for the dropRole. The change is on listPrivilegesByUserName()

Oh sorry got it


- Arjun


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


On June 12, 2018, 6:46 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67555/
> ---
> 
> (Updated June 12, 2018, 6:46 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2267
> https://issues.apache.org/jira/browse/sentry-2267
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Sentry client was failing to get a list of user privileges because the 
> roleName is required.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  f0f08ea3d 
> 
> 
> Diff: https://reviews.apache.org/r/67555/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 67555: SENTRY-2267: Listing user privileges fails because roleName field is required on Thrift

2018-06-12 Thread Arjun Mishra via Review Board 

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


Ship it!




Ship It!

- Arjun Mishra


On June 12, 2018, 6:46 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67555/
> ---
> 
> (Updated June 12, 2018, 6:46 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2267
> https://issues.apache.org/jira/browse/sentry-2267
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Sentry client was failing to get a list of user privileges because the 
> roleName is required.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  f0f08ea3d 
> 
> 
> Diff: https://reviews.apache.org/r/67555/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 67555: SENTRY-2267: Listing user privileges fails because roleName field is required on Thrift

2018-06-12 Thread Sergio Pena via Review Board 


> On June 12, 2018, 6:58 p.m., Arjun Mishra wrote:
> > sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
> > Lines 270 (patched)
> > 
> >
> > Is this just for the dropRole() method?

It's not for the dropRole. The change is on listPrivilegesByUserName()


- Sergio


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


On June 12, 2018, 6:46 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67555/
> ---
> 
> (Updated June 12, 2018, 6:46 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2267
> https://issues.apache.org/jira/browse/sentry-2267
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Sentry client was failing to get a list of user privileges because the 
> roleName is required.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  f0f08ea3d 
> 
> 
> Diff: https://reviews.apache.org/r/67555/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>

Re: Review Request 67555: SENTRY-2267: Listing user privileges fails because roleName field is required on Thrift

2018-06-12 Thread Arjun Mishra via Review Board 

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




sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
Lines 270 (patched)


Is this just for the dropRole() method?


- Arjun Mishra


On June 12, 2018, 6:46 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67555/
> ---
> 
> (Updated June 12, 2018, 6:46 p.m.)
> 
> 
> Review request for sentry, Arjun Mishra, kalyan kumar kalvagadda, and Na Li.
> 
> 
> Bugs: sentry-2267
> https://issues.apache.org/jira/browse/sentry-2267
> 
> 
> Repository: sentry
> 
> 
> Description
> ---
> 
> The Sentry client was failing to get a list of user privileges because the 
> roleName is required.
> 
> 
> Diffs
> -
> 
>   
> sentry-service/sentry-service-api/src/main/java/org/apache/sentry/api/service/thrift/SentryPolicyServiceClientDefaultImpl.java
>  f0f08ea3d 
> 
> 
> Diff: https://reviews.apache.org/r/67555/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>