Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-06 Thread yao lei 


> On 六月 1, 2017, 1:03 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 2708 (patched)
> > 
> >
> > Maybe make this a little clearer:
> > 
> > The directory for scripts which are used by the alert notification 
> > dispatcher.
> 
> yao lei wrote:
> Got it.
> I will make this change.
> 
> yao lei wrote:
> Hi Jonathan Hurley,
> Would you please commit this patch to trunk?
> Thanks.
> 
> Jonathan Hurley wrote:
> Done. Please close the review.

Thanks a lot


- yao


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


On 六月 2, 2017, 1:31 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated 六月 2, 2017, 1:31 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/7/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-06 Thread Jonathan Hurley 


> On June 1, 2017, 9:03 a.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 2708 (patched)
> > 
> >
> > Maybe make this a little clearer:
> > 
> > The directory for scripts which are used by the alert notification 
> > dispatcher.
> 
> yao lei wrote:
> Got it.
> I will make this change.
> 
> yao lei wrote:
> Hi Jonathan Hurley,
> Would you please commit this patch to trunk?
> Thanks.

Done. Please close the review.


- Jonathan


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


On June 1, 2017, 9:31 p.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated June 1, 2017, 9:31 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/7/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-05 Thread yao lei 


> On 六月 5, 2017, 6:21 p.m., Nate Cole wrote:
> > Ship It!

Thanks for your review


- yao


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


On 六月 2, 2017, 1:31 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated 六月 2, 2017, 1:31 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/7/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-05 Thread Nate Cole 

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


Ship it!




Ship It!

- Nate Cole


On June 1, 2017, 9:31 p.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated June 1, 2017, 9:31 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/7/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-01 Thread yao lei 

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

(Updated 六月 2, 2017, 1:31 a.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, and 
Tim Thorpe.


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


Repository: ambari


Description
---

This patch will support using property ambari.dispatch-property.script.filename 
in alert target to tell AlertScriptDispatcher to lookup script by 
filename,default in  directory /var/lib/ambari-server/resources/scripts. 

We can also change this directory in ambari.properties by 
notification.dispatch.alert.script.directory property.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 114046f 
  
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
 84bfe52 
  
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
 f1f320d 


Diff: https://reviews.apache.org/r/59440/diff/7/

Changes: https://reviews.apache.org/r/59440/diff/6-7/


Testing
---

Tested in a cluster


Thanks,

yao lei

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-01 Thread yao lei 


> On 六月 1, 2017, 1:03 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
> > Lines 2708 (patched)
> > 
> >
> > Maybe make this a little clearer:
> > 
> > The directory for scripts which are used by the alert notification 
> > dispatcher.

Got it.
I will make this change.


- yao


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


On 六月 1, 2017, 1:37 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated 六月 1, 2017, 1:37 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/6/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-01 Thread Jonathan Hurley 

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


Ship it!





ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
Lines 2708 (patched)


Maybe make this a little clearer:

The directory for scripts which are used by the alert notification 
dispatcher.


- Jonathan Hurley


On May 31, 2017, 9:37 p.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated May 31, 2017, 9:37 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/6/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-01 Thread yao lei 


> On 六月 1, 2017, 12:10 p.m., Tim Thorpe wrote:
> > Ship It!

Thanks a lot


- yao


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


On 六月 1, 2017, 1:37 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated 六月 1, 2017, 1:37 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/6/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-06-01 Thread Tim Thorpe 

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


Ship it!




Ship It!

- Tim Thorpe


On June 1, 2017, 1:37 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated June 1, 2017, 1:37 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, 
> and Tim Thorpe.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
>  114046f 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/6/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-05-30 Thread yao lei 

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

(Updated May 31, 2017, 4:35 a.m.)


Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.


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


Repository: ambari


Description
---

This patch will support using property ambari.dispatch-property.script.filename 
in alert target to tell AlertScriptDispatcher to lookup script by 
filename,default in  directory /var/lib/ambari-server/resources/scripts. 

We can also change this directory in ambari.properties by 
notification.dispatch.alert.script.directory property.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java
 114046f 
  
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
 84bfe52 
  
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
 f1f320d 


Diff: https://reviews.apache.org/r/59440/diff/4/

Changes: https://reviews.apache.org/r/59440/diff/3-4/


Testing
---

Tested in a cluster


Thanks,

yao lei

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-05-30 Thread yao lei 


> On May 30, 2017, 3:28 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
> > Lines 83-86 (patched)
> > 
> >
> > This should be specified in Configuration as a default property of a 
> > new key (similar to how other directories are specified)

Thanks a lot.
This is fixed in updated patch


- yao


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


On May 25, 2017, 6:02 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated May 25, 2017, 6:02 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/3/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-05-30 Thread Jonathan Hurley 

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


Fix it, then Ship it!





ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
Lines 83-86 (patched)


This should be specified in Configuration as a default property of a new 
key (similar to how other directories are specified)


- Jonathan Hurley


On May 25, 2017, 2:02 a.m., yao lei wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59440/
> ---
> 
> (Updated May 25, 2017, 2:02 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.
> 
> 
> Bugs: AMBARI-21122
> https://issues.apache.org/jira/browse/AMBARI-21122
> 
> 
> Repository: ambari
> 
> 
> Description
> ---
> 
> This patch will support using property 
> ambari.dispatch-property.script.filename in alert target to tell 
> AlertScriptDispatcher to lookup script by filename,default in  directory 
> /var/lib/ambari-server/resources/scripts. 
> 
> We can also change this directory in ambari.properties by 
> notification.dispatch.alert.script.directory property.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
>  84bfe52 
>   
> ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
>  f1f320d 
> 
> 
> Diff: https://reviews.apache.org/r/59440/diff/3/
> 
> 
> Testing
> ---
> 
> Tested in a cluster
> 
> 
> Thanks,
> 
> yao lei
> 
>

Re: Review Request 59440: Part One: Specify the script directly in alert target for script-based alert dispatchers

2017-05-25 Thread yao lei 

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

(Updated 五月 25, 2017, 6:02 a.m.)


Review request for Ambari, Alejandro Fernandez and Jonathan Hurley.


Summary (updated)
-

Part One: Specify the script directly in alert target for script-based alert 
dispatchers


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


Repository: ambari


Description (updated)
---

Although we can create the alert target of type ALERT_SCRIPT by web 
ui(AMBARI-18423), we still need access to ambari.properties to add the new 
script and then restart ambari server to make this function take effect.
It is better specify the script directly in alert target rather than in 
ambari.properties.In this way,we also don't need to restart ambari server.

This patch will support using property ambari.dispatch-property.script.filename 
in alert target to tell AlertScriptDispatcher to lookup script by 
filename,default in /var/lib/ambari-server/resources/scripts directory. 
We can also change this directory in ambari.properties by 
ambari.dispatch-property.script.directory property.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcher.java
 d65a11d 
  
ambari-server/src/test/java/org/apache/ambari/server/notifications/dispatchers/AlertScriptDispatcherTest.java
 68e7d02 


Diff: https://reviews.apache.org/r/59440/diff/2/

Changes: https://reviews.apache.org/r/59440/diff/1-2/


Testing (updated)
---

Unit Tests
1.cd ambari-server & mvn test
  Results :
  Failed tests: 
 ExportBlueprintRequestTest.testExport_noConfigs:138 expected:<3> but 
was:<4>
  Tests run: 4990, Failures: 1, Errors: 0, Skipped: 39
 

Some Test Cases

Case 1:
a.Write a shell script named foo.sh and put it in the directory 
/var/lib/ambari-server/resources/scripts,its content is as following:

\#\!/bin/bash
logFile=/var/log/ambari-server/log_script_filename.log
definitionName=$1
definitionLabel=$2
serviceName=$3
alertState=$4 
alertText=$5
alertTimestamp=$6
hostname=$7
echo received $# parameters:  $definitionName, $definitionLabel, $serviceName, 
$alertState ,$alertText ,$alertTimestamp, $hostname  >> $logFile

b.Create an alert target of type ALERT_SCRIPT and set 
ambari.dispatch-property.script.filename=foo.sh.

Post api/v1/alert_targets
{"AlertTarget":{"name":"test","description":"","global":true,"notification_type":"ALERT_SCRIPT","alert_states":["OK","WARNING","CRITICAL","UNKNOWN"],"properties":{"ambari.dispatch-property.script.filename":"foo.sh"}}}

c.Stop or start any service , wait for a moment, we can see the log in 
/var/ambari-server/log_script_filename.log

Case 2:
a.Add 
ambari.dispatch-property.script.directory=/var/lib/ambari-server/resources/scripts/alerts
 to ambari.properties and move foo.sh to this directory.
b.Restart ambari-server and delete /var/ambari-server/log_script_filename.log
c.Stop or start any service, wait for a moment, we can see the log in 
/var/ambari-server/log_script_filename.log

Case 3:
a.Modify previous alert target and add 
ambari.dispatch-property.script=com.mycompany.dispatch.shell.script

PUT api/v1/alert_targets/:id
{"AlertTarget":{"name":"test","description":"","global":true,"notification_type":"ALERT_SCRIPT","alert_states":["OK","WARNING","CRITICAL","UNKNOWN"],"properties":{"ambari.dispatch-property.script":"com.mycompany.dispatch.shell.script","ambari.dispatch-property.script.filename":"foo.sh"}}}

b.Write a shell script named foo2.sh and put it in directory 
/var/lib/ambari-server/resources/scripts,its content is as following:

\#\!/bin/bash
logFile=/var/log/ambari-server/log_script_dispatch_property.log
definitionName=$1
definitionLabel=$2
serviceName=$3
alertState=$4 
alertText=$5
alertTimestamp=$6
hostname=$7

c.Add 
com.mycompany.dispatch.shell.script=/var/lib/ambari-server/resources/scripts/foo2.sh
 to ambari.properties.
d.Restart ambari-server and delete /var/ambari-server/log_script_filename.log
e.Stop or start any service,wait for a moment, we should see log in 
/var/ambari-server/log_script_filename.log and 
/var/log/ambari-server/log_script_dispatch_property.log is not existed.

Case 4:
a.Modify previous target and delete ambari.dispatch-property.script.filename

PUT api/v1/alert_targets/:id
{"AlertTarget":{"name":"test","description":"","global":true,"notification_type":"ALERT_SCRIPT","alert_states":["OK","WARNING","CRITICAL","UNKNOWN"],"properties":{"ambari.dispatch-property.script":"com.mycompany.dispatch.shell.script"}}}

b.Delete /var/ambari-server/log_script_filename.log
c.Stop or start any service , wait for a moment, we should see log in 
/var/log/ambari-server/log_script_dispatch_property.log and 
/var/ambari-server/log_script_filename.log is not existed any more.


Thanks,

yao lei