> On June 1, 2015, 4:34 p.m., Abraham Elmahrek wrote: > > shell/src/main/java/org/apache/sqoop/shell/RevokePrivilegeFunction.java, > > lines 95-106 > > <https://reviews.apache.org/r/34883/diff/1/?file=975565#file975565line95> > > > > Why not do the following: > > 1. Check if resource, resourceType, and action are null. If they are > > null, then perform "revoke all". > > 2. Check if resource is null. If so, then throw an error. > > 3. Check if resourceType is null. If so, then throw an error. > > 4. Check if action is null. If so, then throw an error. > > 5. Create a privilege object and perform revoke command using passed > > information. > > > > This should be more intuitive and provide clearer feedback to the user.
Thanks Abe for review and comments. Have updated the patch according to your comments. - Dian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34883/#review85996 ----------------------------------------------------------- On June 2, 2015, 1:55 a.m., Dian Fu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34883/ > ----------------------------------------------------------- > > (Updated June 2, 2015, 1:55 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > SQOOP2: Revoke all privilege > > > Diffs > ----- > > client/src/main/java/org/apache/sqoop/client/SqoopClient.java d5c4a8a > shell/src/main/java/org/apache/sqoop/shell/RevokePrivilegeFunction.java > 802d206 > > Diff: https://reviews.apache.org/r/34883/diff/ > > > Testing > ------- > > > Thanks, > > Dian Fu > >
