Re: Review Request 54445: SENTRY-1547 - It is possible to create a privilege with all empty fields

[Mat Crocker <ma...@cloudera.com>]

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



minor nit, should be labeled "fields" not "feilds" 

Looks like you handled all the right cases though

- Mat Crocker


On Dec. 14, 2016, 10:23 p.m., kalyan kumar kalvagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54445/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2016, 10:23 p.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Colin Ma, Hao Hao, Vamsee Yarlagadda, and Vadim Spector.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Both the server_name and action feilds are mandatory in previlage request.
> 
> I had to take a different approach in solving the issue based on the review comments. I have added validation logic in SentryPolicyStoreProcessor enforcing the mandatory fields like server_name and action to be present in privilege/privileges added.
> 
> If the validation is failed exception is raised and appropriate error is message is sent.
> 
> 
> Diffs
> -----
> 
>   sentry-service/sentry-service-common/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java 15b339f 
> 
> Diff: https://reviews.apache.org/r/54445/diff/
> 
> 
> Testing
> -------
> 
> I have done unit tests using sentry cli thrift client.
> 
> 
> File Attachments
> ----------------
> 
> SENTRY-1547.002.patch
>   https://reviews.apache.org/media/uploaded/files/2016/12/14/88c4a123-316d-4ef4-8670-ad38d260f8cb__SENTRY-1547.002.patch
> 
> 
> Thanks,
> 
> kalyan kumar kalvagadda
> 
>