You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Jonathan Hurley <jh...@hortonworks.com> on 2015/04/27 20:21:41 UTC

Review Request 33593: Add Support For Script-Based Alert Dispatchers

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

Review request for Ambari, Nate Cole and Tom Beerbower.


Bugs: AMBARI-9919
    https://issues.apache.org/jira/browse/AMBARI-9919


Repository: ambari


Description
-------

Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:

- Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
- Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
- The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
  ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
  ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 

Diff: https://reviews.apache.org/r/33593/diff/


Testing
-------

Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 

mvn clean test


Thanks,

Jonathan Hurley


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33593/
-----------------------------------------------------------

(Updated April 28, 2015, 10:35 a.m.)


Review request for Ambari, Nate Cole and Tom Beerbower.


Changes
-------

Added configurable timeouts for the scripts and their own executor.


Bugs: AMBARI-9919
    https://issues.apache.org/jira/browse/AMBARI-9919


Repository: ambari


Description
-------

Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:

- Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
- Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
- The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
  ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java e79e745 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
  ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 

Diff: https://reviews.apache.org/r/33593/diff/


Testing
-------

Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 

mvn clean test


Thanks,

Jonathan Hurley


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33593/#review81720
-----------------------------------------------------------

Ship it!


Ship It!

- Tom Beerbower


On April 27, 2015, 7:32 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33593/
> -----------------------------------------------------------
> 
> (Updated April 27, 2015, 7:32 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-9919
>     https://issues.apache.org/jira/browse/AMBARI-9919
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:
> 
> - Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
> - Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
> - The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
>   ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 
> 
> Diff: https://reviews.apache.org/r/33593/diff/
> 
> 
> Testing
> -------
> 
> Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On April 28, 2015, 9:23 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java, line 142
> > <https://reviews.apache.org/r/33593/diff/2/?file=943230#file943230line142>
> >
> >     Seems a little dangerous if the process blocks/deadlocks.  Maybe wrap in a thread that you then invoke join(timeout) on?
> >     
> >     Thread t = new Thread(new Runnable() \{
> >       try \{
> >         process.waitFor();
> >       \} catch (Exception e) \{
> >         process.kill();
> >       \}
> >     \});
> >     t.start();
> >     t.join(2000);
> >     
> >     (or something like that, maybe extend thread to capture the exit code?)

Agreed that this could be a problem. Implemented a timeout policy along with an executor.


- Jonathan


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


On April 27, 2015, 3:32 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33593/
> -----------------------------------------------------------
> 
> (Updated April 27, 2015, 3:32 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-9919
>     https://issues.apache.org/jira/browse/AMBARI-9919
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:
> 
> - Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
> - Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
> - The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
>   ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 
> 
> Diff: https://reviews.apache.org/r/33593/diff/
> 
> 
> Testing
> -------
> 
> Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33593/#review81811
-----------------------------------------------------------

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
<https://reviews.apache.org/r/33593/#comment132304>

    Seems a little dangerous if the process blocks/deadlocks.  Maybe wrap in a thread that you then invoke join(timeout) on?
    
    Thread t = new Thread(new Runnable() \{
      try \{
        process.waitFor();
      \} catch (Exception e) \{
        process.kill();
      \}
    \});
    t.start();
    t.join(2000);
    
    (or something like that, maybe extend thread to capture the exit code?)


- Nate Cole


On April 27, 2015, 3:32 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33593/
> -----------------------------------------------------------
> 
> (Updated April 27, 2015, 3:32 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-9919
>     https://issues.apache.org/jira/browse/AMBARI-9919
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:
> 
> - Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
> - Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
> - The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
>   ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 
> 
> Diff: https://reviews.apache.org/r/33593/diff/
> 
> 
> Testing
> -------
> 
> Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33593/
-----------------------------------------------------------

(Updated April 27, 2015, 3:32 p.m.)


Review request for Ambari, Nate Cole and Tom Beerbower.


Bugs: AMBARI-9919
    https://issues.apache.org/jira/browse/AMBARI-9919


Repository: ambari


Description
-------

Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:

- Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
- Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
- The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
  ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
  ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
  ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
  ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
  ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
  ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 

Diff: https://reviews.apache.org/r/33593/diff/


Testing
-------

Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 

mvn clean test


Thanks,

Jonathan Hurley


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On April 27, 2015, 2:42 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java, line 448
> > <https://reviews.apache.org/r/33593/diff/1/?file=943126#file943126line448>
> >
> >     Is the added dispatcher parameter needed here?  It doesn't appear to be used.

It actually is needed; looks like I missed an if-statement that said if the dispatcher doesn't need content generated, don't generate content. The code as it exists today would fall back to using a basic formatter to generate the content, which would then be ignored by the dispatcher. I'll fix this up.


> On April 27, 2015, 2:42 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java, line 513
> > <https://reviews.apache.org/r/33593/diff/1/?file=943126#file943126line513>
> >
> >     Same as above... is dispatcher needed?

Same as above.


- Jonathan


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


On April 27, 2015, 2:21 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33593/
> -----------------------------------------------------------
> 
> (Updated April 27, 2015, 2:21 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-9919
>     https://issues.apache.org/jira/browse/AMBARI-9919
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:
> 
> - Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
> - Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
> - The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
>   ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 
> 
> Diff: https://reviews.apache.org/r/33593/diff/
> 
> 
> Testing
> -------
> 
> Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>


Re: Review Request 33593: Add Support For Script-Based Alert Dispatchers

Posted by Tom Beerbower <tb...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33593/#review81712
-----------------------------------------------------------

Ship it!


Looks good... a couple comments.


ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
<https://reviews.apache.org/r/33593/#comment132134>

    Is the added dispatcher parameter needed here?  It doesn't appear to be used.



ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java
<https://reviews.apache.org/r/33593/#comment132135>

    Same as above... is dispatcher needed?


- Tom Beerbower


On April 27, 2015, 6:21 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33593/
> -----------------------------------------------------------
> 
> (Updated April 27, 2015, 6:21 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-9919
>     https://issues.apache.org/jira/browse/AMBARI-9919
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add support for alert dispatchers which invoke a configurable command line script. This involves several steps:
> 
> - Change the Ambari framework to allow for custom dispatchers to be picked up via the classpath.
> - Allow for dispatchers to receive notifications that do not have pre-generated content since the {{alert-templates.xml}} may not be able to provide the correct command-line arguments for the script.
> - The script dispatcher should be able to use any script defined in ambari.properties. A single dispatcher class can therefore dispatch to any number of scripts if the script path is found in ambari.properties.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java bbeca38 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java 184c8db 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/Notification.java 5e34b12 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/NotificationDispatcher.java 8a88b42 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/EmailDispatcher.java 4abec3a 
>   ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/SNMPDispatcher.java a147bac 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertNotification.java PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java 9f3b5d8 
>   ambari-server/src/main/java/org/apache/ambari/server/state/services/AlertNoticeDispatchService.java 0f0e637 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/DispatchFactoryTest.java ff626ac 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/MockDispatcher.java 5c248e1 
>   ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java PRE-CREATION 
>   ambari-server/src/test/java/org/apache/ambari/server/state/services/AlertNoticeDispatchServiceTest.java c4c2ed8 
> 
> Diff: https://reviews.apache.org/r/33593/diff/
> 
> 
> Testing
> -------
> 
> Added a dummy script to ambari.properties and had it echo out the alert properties that were being sent. 
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>