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