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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/alerts/AgentHeartbeatAlertRunnable.java
 (lines 20 - 21)
<https://reviews.apache.org/r/44923/#comment186221>

    No changes in this file; just import moving, so does it need to be in the 
commit?
    
    Also, defaults should be:
    ```
    java
    org
    com
    ```



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java
 (line 560)
<https://reviews.apache.org/r/44923/#comment186222>

    StringUtils.equals() is easier to read (and safer)



ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
 (line 50)
<https://reviews.apache.org/r/44923/#comment186232>

    Our serialized JSON uses _ instead of camelCase (see ignoreHost).
    
    We should add @SerializedName("help_url") here.



ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
 (lines 305 - 311)
<https://reviews.apache.org/r/44923/#comment186224>

    StringUtils.equals() cuts the lines in half.



ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
 (lines 20 - 33)
<https://reviews.apache.org/r/44923/#comment186225>

    Can you adjust your imports so they are:
    ```
    java
    org
    com
    ```



ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java
 (line 447)
<https://reviews.apache.org/r/44923/#comment186228>

    You have the size defined as 512 in the entity, but 255 here.
    
    I'm fine with either size, but we should be consistent.


- Jonathan Hurley


On March 16, 2016, 3:43 p.m., Vitalyi Brodetskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44923/
> -----------------------------------------------------------
> 
> (Updated March 16, 2016, 3:43 p.m.)
> 
> 
> Review request for Ambari, Dmytro Sen and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-15445
>     https://issues.apache.org/jira/browse/AMBARI-15445
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> For some alerts, having the ability to include a link in the response would 
> be incredibly beneficial.  Having a structured response format would allow 
> alert creators to add links that operators could click on to help give more 
> context around an alert, or point to a remediation step.  Something like:
> 
> {code}
> {
>   "response": "There were more than 5 soft limits reached in the past 10 
> minutes.  Please click the link to see the log file.",
>   "href": "http://some.server/other/stuff";
> }
> {code}
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/alerts/AgentHeartbeatAlertRunnable.java
>  0611e23 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java
>  0f73ec6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertDefinitionEntity.java
>  9e21bec 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/AlertDefinitionEntity_.java
>  d2d4cf3 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
>  9fff5f2 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
>  3b4f5fc 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java
>  a803f73 
>   ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql a85202d 
>   ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 9b4810c 
>   ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql cc3d197 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 07c786d 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 
> ab6dc93 
>   ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 8e91fde 
>   ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 440ca44 
> 
> Diff: https://reviews.apache.org/r/44923/diff/
> 
> 
> Testing
> -------
> 
> Tests will be added after patch draft review
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Reply via email to