You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Vitalyi Brodetskyi <vb...@hortonworks.com> on 2016/03/16 20:43:19 UTC
Review Request 44923: Alerts: Allow alerts to emit a structured
response
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44923/
-----------------------------------------------------------
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
Posted by Vitalyi Brodetskyi <vb...@hortonworks.com>.
> On Березень 16, 2016, 8:04 після полудня, Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java, line 569
> > <https://reviews.apache.org/r/44923/diff/1/?file=1301730#file1301730line569>
> >
> > StringUtils.equals() is easier to read (and safer)
We can't use StringUtils.equals() here because of (null != helpURL), in the same time StringUtils equals method don't have such part of logic.
- Vitalyi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44923/#review123914
-----------------------------------------------------------
On Березень 16, 2016, 7:43 після полудня, Vitalyi Brodetskyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44923/
> -----------------------------------------------------------
>
> (Updated Березень 16, 2016, 7:43 після полудня)
>
>
> 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
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 44923: Alerts: Allow alerts to emit a structured
response
Posted by Vitalyi Brodetskyi <vb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44923/#review123912
-----------------------------------------------------------
Tests will be added after review of patch draft(current patch).
- Vitalyi Brodetskyi
On Березень 16, 2016, 7:43 після полудня, Vitalyi Brodetskyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44923/
> -----------------------------------------------------------
>
> (Updated Березень 16, 2016, 7:43 після полудня)
>
>
> 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
Posted by Vitalyi Brodetskyi <vb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44923/
-----------------------------------------------------------
(Updated Березень 17, 2016, 9:28 після полудня)
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
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProviderTest.java d44b7c8
ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java 7f3b80c
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
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44923/#review124089
-----------------------------------------------------------
Ship it!
Ship It!
- Jonathan Hurley
On March 17, 2016, 4:11 p.m., Vitalyi Brodetskyi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44923/
> -----------------------------------------------------------
>
> (Updated March 17, 2016, 4:11 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/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
Posted by Vitalyi Brodetskyi <vb...@hortonworks.com>.
-----------------------------------------------------------
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