Re: Review Request 44923: Alerts: Allow alerts to emit a structured response

2016-03-19 Thread Jonathan Hurley

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


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)


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



ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinition.java
 (line 50)


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)


StringUtils.equals() cuts the lines in half.



ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionFactory.java
 (lines 20 - 33)


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)


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



Re: Review Request 44923: Alerts: Allow alerts to emit a structured response

2016-03-19 Thread Vitalyi Brodetskyi

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

(Updated Березень 17, 2016, 8:11 після полудня)


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 (updated)
-

  
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