You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Yurii Shylov <yu...@gmail.com> on 2015/01/02 19:06:58 UTC
Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/
-----------------------------------------------------------
Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
Bugs: AMBARI-8978
https://issues.apache.org/jira/browse/AMBARI-8978
Repository: ambari
Description
-------
During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
Diffs
-----
ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
Diff: https://reviews.apache.org/r/29547/diff/
Testing
-------
In progress
Thanks,
Yurii Shylov
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review67281
-----------------------------------------------------------
Ship it!
Ship It!
- Tom Beerbower
On Jan. 8, 2015, 6:23 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 6:23 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review67285
-----------------------------------------------------------
Ship it!
Looks good; I think we just have to make sure we only validate when the property exists and is true.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
<https://reviews.apache.org/r/29547/#comment111283>
I think that absence of the validation property would mean we don't validate. Only if it was present and true would validation occur.
- Jonathan Hurley
On Jan. 8, 2015, 1:23 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 8, 2015, 1:23 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review67432
-----------------------------------------------------------
Ship it!
Ship It!
- Tom Beerbower
On Jan. 9, 2015, 1:45 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 1:45 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review67453
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Jan. 9, 2015, 8:45 a.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 8:45 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review67880
-----------------------------------------------------------
Committed; please close this review.
- Jonathan Hurley
On Jan. 9, 2015, 8:45 a.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2015, 8:45 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Yurii Shylov <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/
-----------------------------------------------------------
(Updated Янв. 9, 2015, 1:45 п.п.)
Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
Changes
-------
Fixed incorrect use of validation property
Bugs: AMBARI-8978
https://issues.apache.org/jira/browse/AMBARI-8978
Repository: ambari
Description
-------
During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
Diff: https://reviews.apache.org/r/29547/diff/
Testing
-------
Results :
Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
Thanks,
Yurii Shylov
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Yurii Shylov <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/
-----------------------------------------------------------
(Updated Янв. 8, 2015, 6:23 п.п.)
Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
Changes
-------
Patch is updated according to Tom's comments
Bugs: AMBARI-8978
https://issues.apache.org/jira/browse/AMBARI-8978
Repository: ambari
Description
-------
During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java bc0d81d
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java 197e3ec
ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
Diff: https://reviews.apache.org/r/29547/diff/
Testing
-------
Results :
Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
Thanks,
Yurii Shylov
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Yurii Shylov <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/
-----------------------------------------------------------
(Updated Янв. 5, 2015, 5:13 п.п.)
Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
Changes
-------
Updated according to comments, fixed conflict with latest trunk
Bugs: AMBARI-8978
https://issues.apache.org/jira/browse/AMBARI-8978
Repository: ambari
Description
-------
During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
Diff: https://reviews.apache.org/r/29547/diff/
Testing (updated)
-------
Results :
Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
Thanks,
Yurii Shylov
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66645
-----------------------------------------------------------
Ship it!
Mostly minor comments. One or two issues to correct before shipping.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java
<https://reviews.apache.org/r/29547/#comment110210>
Null pointer check on `dispatchFactory.getDispatcher(notificationType)` since there's no way to ensure that the correct dispatcher will exist.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java
<https://reviews.apache.org/r/29547/#comment110211>
I think for enums, we use `.name()` instead of `.toString()`
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java
<https://reviews.apache.org/r/29547/#comment110212>
Ambari doesn't use `.*` for imports.
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java
<https://reviews.apache.org/r/29547/#comment110213>
Because this is method designed for testing the validity of a target, exceptions are expected. However, I think we still need to either:
1) Capture the entire exception in the result so it's available if needed
or
2) Log the exception to some non-critical level like DEBUG.
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java
<https://reviews.apache.org/r/29547/#comment110214>
`AMBARI_DISPATCH_CREDENTIAL_USERNAME` is not a requirement for connection. Only if it's specified as part of the target properties should a `DispatchCredentials` instance be created.
- Jonathan Hurley
On Jan. 2, 2015, 1:06 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2015, 1:06 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> In progress
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Jonathan Hurley <jh...@hortonworks.com>.
> On Jan. 5, 2015, 10:25 a.m., Tom Beerbower wrote:
> > A general question about this proposal... why couldn't the user create an alert target in some "test" mode where the target is not really active until the user is satisfied that the target is configured correctly? At that point the user could flip the switch from "test" to "active". I think that the new resource that you are proposing gets created on a POST then discarded right away (you can never GET it) which is not really in line with the rest of the APIs.
I actually went back and forth over this as well. I thought that adding this logic to the alert target resource provider didn't seem right as it was only to be used for validation of a target's connection capability. The alert target resource provider should only really be providing CRUD operations on a new or existing target. We didn't actually want a target to be created.
Nate brought up the point that we could use a property on the POST to control this, but it just feels like we'd be over-complicating the POST in that case. I dislike the idea of a buried property altering the entire behavior of a REST method.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66651
-----------------------------------------------------------
On Jan. 2, 2015, 1:06 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2015, 1:06 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> In progress
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by John Speidel <js...@hortonworks.com>.
> On Jan. 5, 2015, 3:25 p.m., Tom Beerbower wrote:
> > A general question about this proposal... why couldn't the user create an alert target in some "test" mode where the target is not really active until the user is satisfied that the target is configured correctly? At that point the user could flip the switch from "test" to "active". I think that the new resource that you are proposing gets created on a POST then discarded right away (you can never GET it) which is not really in line with the rest of the APIs.
>
> Jonathan Hurley wrote:
> I actually went back and forth over this as well. I thought that adding this logic to the alert target resource provider didn't seem right as it was only to be used for validation of a target's connection capability. The alert target resource provider should only really be providing CRUD operations on a new or existing target. We didn't actually want a target to be created.
>
> Nate brought up the point that we could use a property on the POST to control this, but it just feels like we'd be over-complicating the POST in that case. I dislike the idea of a buried property altering the entire behavior of a REST method.
>
> Tom Beerbower wrote:
> Ok, I see. I would still like to find a solution that is in line with existing APIs. The create blueprint API allows you to do ...
>
> POST blueprints/myBP?validate_topology={true/false}
>
> where if you specify validate_topology=true and the validation fails then the BP is not created. Could we do something along those lines for alert targets?
>
> John Speidel wrote:
> +1 for Tom's suggestion.
In your resource definition, you can override the method: Collection<String> getCreateDirectives() and specify a create directive like "validate_connection".
This property and it's value will be made available to the resource provider via the request info properties which can be obtained from the request: request.getRequestInfoProperties()
When making a request, the user may specify the optional create directive as Tom mentioned above: myResource?validate_connection=true
See BlueprintResourceDefinition and BlueprintResourceProvider for examples.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66651
-----------------------------------------------------------
On Jan. 5, 2015, 5:13 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2015, 5:13 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by John Speidel <js...@hortonworks.com>.
> On Jan. 5, 2015, 3:25 p.m., Tom Beerbower wrote:
> > A general question about this proposal... why couldn't the user create an alert target in some "test" mode where the target is not really active until the user is satisfied that the target is configured correctly? At that point the user could flip the switch from "test" to "active". I think that the new resource that you are proposing gets created on a POST then discarded right away (you can never GET it) which is not really in line with the rest of the APIs.
>
> Jonathan Hurley wrote:
> I actually went back and forth over this as well. I thought that adding this logic to the alert target resource provider didn't seem right as it was only to be used for validation of a target's connection capability. The alert target resource provider should only really be providing CRUD operations on a new or existing target. We didn't actually want a target to be created.
>
> Nate brought up the point that we could use a property on the POST to control this, but it just feels like we'd be over-complicating the POST in that case. I dislike the idea of a buried property altering the entire behavior of a REST method.
>
> Tom Beerbower wrote:
> Ok, I see. I would still like to find a solution that is in line with existing APIs. The create blueprint API allows you to do ...
>
> POST blueprints/myBP?validate_topology={true/false}
>
> where if you specify validate_topology=true and the validation fails then the BP is not created. Could we do something along those lines for alert targets?
+1 for Tom's suggestion.
- John
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66651
-----------------------------------------------------------
On Jan. 5, 2015, 5:13 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2015, 5:13 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java d8c1fda
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 289e594
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 6a14f1b
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 626799a
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java c88427d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 1dc7e2d
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 1feb102
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> Results :
>
> Tests run: 2486, Failures: 0, Errors: 0, Skipped: 13
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Tom Beerbower <tb...@hortonworks.com>.
> On Jan. 5, 2015, 3:25 p.m., Tom Beerbower wrote:
> > A general question about this proposal... why couldn't the user create an alert target in some "test" mode where the target is not really active until the user is satisfied that the target is configured correctly? At that point the user could flip the switch from "test" to "active". I think that the new resource that you are proposing gets created on a POST then discarded right away (you can never GET it) which is not really in line with the rest of the APIs.
>
> Jonathan Hurley wrote:
> I actually went back and forth over this as well. I thought that adding this logic to the alert target resource provider didn't seem right as it was only to be used for validation of a target's connection capability. The alert target resource provider should only really be providing CRUD operations on a new or existing target. We didn't actually want a target to be created.
>
> Nate brought up the point that we could use a property on the POST to control this, but it just feels like we'd be over-complicating the POST in that case. I dislike the idea of a buried property altering the entire behavior of a REST method.
Ok, I see. I would still like to find a solution that is in line with existing APIs. The create blueprint API allows you to do ...
POST blueprints/myBP?validate_topology={true/false}
where if you specify validate_topology=true and the validation fails then the BP is not created. Could we do something along those lines for alert targets?
- Tom
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66651
-----------------------------------------------------------
On Jan. 2, 2015, 6:06 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2015, 6:06 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> In progress
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66651
-----------------------------------------------------------
A general question about this proposal... why couldn't the user create an alert target in some "test" mode where the target is not really active until the user is satisfied that the target is configured correctly? At that point the user could flip the switch from "test" to "active". I think that the new resource that you are proposing gets created on a POST then discarded right away (you can never GET it) which is not really in line with the rest of the APIs.
- Tom Beerbower
On Jan. 2, 2015, 6:06 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2015, 6:06 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> In progress
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66653
-----------------------------------------------------------
Ship it!
Ship It!
- Nate Cole
On Jan. 2, 2015, 1:06 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2015, 1:06 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> In progress
>
>
> Thanks,
>
> Yurii Shylov
>
>
Re: Review Request 29547: Alerts: Allow Ability To Test An AlertTarget
Before Creating It
Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29547/#review66642
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java
<https://reviews.apache.org/r/29547/#comment110208>
Instead of this new class, just use SimpleResourceDefinition.
ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
<https://reviews.apache.org/r/29547/#comment110209>
Use a SimpleResourceDefinition.
- Nate Cole
On Jan. 2, 2015, 1:06 p.m., Yurii Shylov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29547/
> -----------------------------------------------------------
>
> (Updated Jan. 2, 2015, 1:06 p.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Nate Cole, and Tom Beerbower.
>
>
> Bugs: AMBARI-8978
> https://issues.apache.org/jira/browse/AMBARI-8978
>
>
> Repository: ambari
>
>
> Description
> -------
>
> During the cluster installation, the web client would like to be able to have the administrator configure an alert target for use with that cluster. However, because there are many properties that are used to successfully create an AlertTarget, it's likely that the settings originally provided may not work.
>
> For example, when creating an AlertTarget for SMTP, if the security or port are not valid (or the SMTP server is restricting access to certain IP addresses) then the target won't be able to properly use it.
>
> We need to be able to allow an AlertTarget to be "tested" before actually creating it in the system.
>
> I propose a new endpoint off of targets that can be used to POST to. The POST can contain all of the alert properties that would normally be found on an AlertTarget. The difference is that no target is created; instead a status is returned about whether the target works (and why it doesn't if it failed).
>
> I would suggest also altering the dispatcher interface to support a new method; something like {{Dispatcher.testAlertTarget(...)}} which will simply exercise the properties of the target to ensure a good connection.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetValidationResourceDefinition.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java e55b2cb
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertTargetValidationService.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProvider.java PRE-CREATION
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java be2a9ad
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java 89d6837
> ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 10946be
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java f979c03
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java 0e75801
> ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java b085112
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetValidationResourceProviderTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/EmailDispatcherTest.java 1e7689f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 616551f
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcherTest.java PRE-CREATION
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcherTest.java db4af1c
> ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java 2e984bf
>
> Diff: https://reviews.apache.org/r/29547/diff/
>
>
> Testing
> -------
>
> In progress
>
>
> Thanks,
>
> Yurii Shylov
>
>