> On Aug. 26, 2014, 7:51 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertingService.java,
> >  line 42
> > <https://reviews.apache.org/r/25071/diff/1/?file=669512#file669512line42>
> >
> >     I think that there is an issue here that /alerting/...  doesn't fit the 
> > basic format that we use for the API.  Basically ...
> >     
> >         /api/v1/[plural resource type]/[resource id]/[plural sub-resource 
> > type]/[sub-resource id]/...
> >     
> >     Would it be possible to change from ...
> >     
> >         .../alerting/groups/{id}
> >         .../alerting/targets/{id}
> >     
> >     to ...
> >     
> >         .../alert_groups/{id}
> >         .../alert_targets/{id}
> >     
> >     ? Or is there some reason that we need the extra level?  I think that 
> > we already have an alert_definition resource.  Wouldn't alert_group and 
> > alert_target be consistent with that?
> 
> Jonathan Hurley wrote:
>     The original idea was to have more than just groups exposed under 
> "v1/clusters/{cluster}/alerting"; we'd also have history exposed there as 
> well. But I don't see any reason why we couldn't just keep consistent here 
> and have alert_groups instead (and alert_history later on).
>     
>     Targets are a little difference since they are not bound to a cluster. In 
> this case, I think it makes sense to put them under "alerting"; if we want to 
> be a little more consistent and have a plural name, we could make this 
> v1/alerts/targets (and later on v1/alerts/history for cross-cluster history). 
> Thoughts on this versus v1/alert_targets ?

I think that v1/alerts/targets and v1/alerts/history still have the same issue 
of not fitting the format.

We have other resources that appear at both the cluster and root levels so it 
shouldn't matter if targets are bound to a cluster or not.  Could it be ...

v1/alert_targets and v1/alert_histories ?

Otherwise, what would the response be for v1/alerts?  Would they get a set of 
alerts back or is that just a category to group targets and histories?


- Tom


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


On Aug. 26, 2014, 6:25 p.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25071/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2014, 6:25 p.m.)
> 
> 
> Review request for Ambari, Nate Cole and Tom Beerbower.
> 
> 
> Bugs: AMBARI-7021
>     https://issues.apache.org/jira/browse/AMBARI-7021
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Add the initial REST endpoints for interacting with alert targets and groups. 
> 
> Update is not implemented since there's the larger issue of how to tackle 
> generic concepts like target properties. Also, discussion needs to be around 
> what default data gets returned in the queries (like targets and definitions) 
> - right now the minimum data is returned.
> 
> 
> Diffs
> -----
> 
>   ambari-agent/src/main/python/ambari_agent/alerts/base_alert.py 
> 07987d91c19a13c7a582d506945bde607c74bb81 
>   ambari-agent/src/main/python/ambari_agent/alerts/port_alert.py 
> d06d1f40daeb57a7db848a465382221e765c067c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
>  dca3bd92eedfc08cfd10ffe1056ca32327e12220 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertGroupResourceDefinition.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/AlertTargetResourceDefinition.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
>  13fff1d5d9d12cc1b0fa3ea62a773005a280e4d9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertGroupService.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertingService.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java
>  8043b3fb567b48eff682d07f978f678099d4ebc2 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java
>  21c9d80732b05c0b09206ceb9578a23a36aa6e14 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractControllerResourceProvider.java
>  71ddc8d9f6579a05e82536943d25665537558f29 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java
>  b10b4bca27d2a52bd0730f5c3a2380dffcb54ff5 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/AlertDispatchDAO.java
>  2871e62f92c9767ad7b8006dd552861642896b83 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertGroup.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertTarget.java
>  PRE-CREATION 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/alert/TargetType.java
>  PRE-CREATION 
>   ambari-server/src/main/resources/key_properties.json 
> 06ebb615d7d5256db432ca012b97608cd948ebe6 
>   ambari-server/src/main/resources/properties.json 
> bc2ad22691e24e339ad1b7ffe9b1d7bf3dca8306 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProviderTest.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25071/diff/
> 
> 
> Testing
> -------
> 
> 
> Results :
> 
> Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Reply via email to