----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41358/#review110957 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java (lines 523 - 524) <https://reviews.apache.org/r/41358/#comment170972> If you're in this method, pretty much "managed" is always true. The only time it's not is if you're toggling the definition off. So, why not make this simpler and not have every if-statement set managed. Just unset it in the enable if-statement. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java (line 581) <https://reviews.apache.org/r/41358/#comment170971> JSON could be slightly different and still be the same - Why do a comparison here? ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java (line 233) <https://reviews.apache.org/r/41358/#comment170973> Wrong log statement; you're deleting the group here, not a target. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java (line 162) <https://reviews.apache.org/r/41358/#comment170974> I still see some of these checks on a per-entity basis. Here and above in the definition resource provider. Considering that there could be 1000's of alerts here, can't we do this outside of the loop? - Jonathan Hurley On Dec. 16, 2015, 1:58 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41358/ > ----------------------------------------------------------- > > (Updated Dec. 16, 2015, 1:58 p.m.) > > > Review request for Ambari, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, > and Sid Wagle. > > > Bugs: AMBARI-14141 > https://issues.apache.org/jira/browse/AMBARI-14141 > > > Repository: ambari > > > Description > ------- > > Enforce granular role-based access control for alert functions: > > | Cluster User | Service Operator | Service > Administrator | Cluster Operator | Cluster Administrator | Administrator > --------------------------------|--------------|------------------|-----------------------|------------------|-----------------------|--------------- > > View alerts (service) | (+) | (+) | (+) > | (+) | (+) | (+) > Enable/disable alerts (service) | | | (+) > | (+) | (+) | (+) > View alerts (cluster) | (+) | (+) | (+) > | (+) | (+) | (+) > Enable/disable alerts (cluster) | | | > | | (+) | (+) > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java > a29f151 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java > bc5f956 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java > 215bc8e > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProvider.java > 89ee69a > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProvider.java > 8f0e526 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProvider.java > 4dc4dcf > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertResourceProviderUtils.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java > a310259 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java > dde934d > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java > d817ad7 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/RoleAuthorization.java > 795db77 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog230.java > 57eafa6 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 4a980ec > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 60bbd30 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql f1fb358 > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 1d9cc71 > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 55846c0 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 9f289bc > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProviderTest.java > e589537 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java > a41eecf > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertHistoryResourceProviderTest.java > 99aca45 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertNoticeResourceProviderTest.java > 3322da6 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertResourceProviderTest.java > 4f0263b > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java > 6cde0c2 > > ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java > 1c440eb > > Diff: https://reviews.apache.org/r/41358/diff/ > > > Testing > ------- > > Manually tested > > # Local test results: > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 1:00:59.413s > [INFO] Finished at: Mon Dec 14 13:37:40 EST 2015 > [INFO] Final Memory: 70M/1085M > [INFO] > ------------------------------------------------------------------------ > > # Jenkins test results: PENDING > > > Thanks, > > Robert Levas > >
