----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30266/#review70321 -----------------------------------------------------------
Looks cool to me. A couple of thoughts. security/src/main/java/org/apache/sqoop/security/Authorization/AuthorizationEngine.java <https://reviews.apache.org/r/30266/#comment115435> Potentially make methods non static so that they are mockable. I don't think it matters so much in the current implementation. security/src/main/java/org/apache/sqoop/security/Authorization/AuthorizationEngine.java <https://reviews.apache.org/r/30266/#comment115432> Avoid exception handling for control flow? Note on style: potentially use java list filters instead. Big NIT though. security/src/main/java/org/apache/sqoop/security/Authorization/AuthorizationEngine.java <https://reviews.apache.org/r/30266/#comment115427> Create privileges seem to use empty strings as resource IDs. Is the resource ID arbitrary and can be any thing? If so, it might make sense to make this a configurable string. security/src/main/java/org/apache/sqoop/security/Authorization/AuthorizationEngine.java <https://reviews.apache.org/r/30266/#comment115433> return boolean? - Abraham Elmahrek On Jan. 27, 2015, 3:55 a.m., richard zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30266/ > ----------------------------------------------------------- > > (Updated Jan. 27, 2015, 3:55 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-sqoop2 > > > Description > ------- > > RBAC engine, which handles all rules. Such as, create a job needs global > create job privilege and use related link privilege. > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/security/SecurityError.java > c68b666c83212d91e183d8041e694629a62b7224 > > security/src/main/java/org/apache/sqoop/security/Authorization/AuthorizationEngine.java > PRE-CREATION > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > ccf928eb0efbb2137b9269db03ee444f51c034e6 > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > f4482cee7459febb37bb747a011f611655412783 > server/src/main/java/org/apache/sqoop/handler/LinkRequestHandler.java > 74fa321a57468e9b75b75213f0ee20ed708c8ead > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > a0b29c86b7b7d7fe8d5fc185164c7899050083ca > > Diff: https://reviews.apache.org/r/30266/diff/ > > > Testing > ------- > > local + integration test > > > Thanks, > > richard zhou > >
