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

Reply via email to