> On Dec. 17, 2015, 9:20 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java,
> >  lines 523-524
> > <https://reviews.apache.org/r/41358/diff/2/?file=1166688#file1166688line523>
> >
> >     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.
> 
> Robert Levas wrote:
>     Then it wont be clear if the only thing that is being changed is the 
> enabled/disabled flag. For how do I know the difference between the alert is 
> being enabled and scheduled or just enabled?

Check the request map to see what's a part of the request? Unless the web 
client always sends down the entire JSON structure even when only changing 1 
field.


- Jonathan


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


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
> 
>

Reply via email to