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 2014/08/26 20:25:46 UTC

Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

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

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

> On Aug. 27, 2014, 8:57 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java, line 793
> > <https://reviews.apache.org/r/25071/diff/2/?file=669875#file669875line793>
> >
> >     How is a registration alert definition command different from the other kind?

This method should only be invoked when registering a host (handling a registration heartbeat). Since both regular and registration heartbeats are handled in this class, I felt it was good form to change the name and update the documentation to reflect that this method is only to get the commands required for a registering host. This actually invalidates the hosts as well, so it's not something that should be invoked in normal heartbeat responses.


> On Aug. 27, 2014, 8:57 a.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/base_alert.py, line 37
> > <https://reviews.apache.org/r/25071/diff/2/?file=669873#file669873line37>
> >
> >     Petty.  We don't use camel case in python.  If anything this should be host_name.

Ugh; point taken. However, I did want to spell out it was the name of the host (to go along with cluster_name in other areas). I've made the naming change.


> On Aug. 27, 2014, 8:57 a.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/port_alert.py, line 79
> > <https://reviews.apache.org/r/25071/diff/2/?file=669874#file669874line79>
> >
> >     Why make this change?  This utility method could be used other places, but marking it with self means that now an instance is required to use it.   Also, the call syntax is wrong in that now you need an implied self.get_host_from_url

It was defined in base_alert and to me that says that only concrete alerts would use it. We have a weird scenario where if only a port is given for the URI, we need to assume that current host is the host. This method would previously return the port instead of an actual host.

The host is stored in "self.hostName". In order to access it from this method, we need a reference to self. I thought python always passes self automatically if declared in the method.


- Jonathan


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


On Aug. 26, 2014, 8:17 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, 8:17 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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
>   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 
>   ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 
> 
> Diff: https://reviews.apache.org/r/25071/diff/
> 
> 
> Testing
> -------
> 
> Results :
> Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14
> 
> ** Note: Found a race condition issue with a test case that was causing inconsistent failures; this is what the change to ServiceComponentHostImpl/Test is **
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

> On Aug. 27, 2014, 8:57 a.m., Nate Cole wrote:
> > ambari-agent/src/main/python/ambari_agent/alerts/port_alert.py, line 79
> > <https://reviews.apache.org/r/25071/diff/2/?file=669874#file669874line79>
> >
> >     Why make this change?  This utility method could be used other places, but marking it with self means that now an instance is required to use it.   Also, the call syntax is wrong in that now you need an implied self.get_host_from_url
> 
> Jonathan Hurley wrote:
>     It was defined in base_alert and to me that says that only concrete alerts would use it. We have a weird scenario where if only a port is given for the URI, we need to assume that current host is the host. This method would previously return the port instead of an actual host.
>     
>     The host is stored in "self.hostName". In order to access it from this method, we need a reference to self. I thought python always passes self automatically if declared in the method.

After more discussion, we will make this method static again and return None when a numeric is passed in. The subclasses will have to check for None and then determine what to do (in port_alert's case, it will default to the self.host_name).


- Jonathan


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


On Aug. 26, 2014, 8:17 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, 8:17 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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
>   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 
>   ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 
> 
> Diff: https://reviews.apache.org/r/25071/diff/
> 
> 
> Testing
> -------
> 
> Results :
> Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14
> 
> ** Note: Found a race condition issue with a test case that was causing inconsistent failures; this is what the change to ServiceComponentHostImpl/Test is **
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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



ambari-agent/src/main/python/ambari_agent/alerts/base_alert.py
<https://reviews.apache.org/r/25071/#comment90153>

    Petty.  We don't use camel case in python.  If anything this should be host_name.



ambari-agent/src/main/python/ambari_agent/alerts/port_alert.py
<https://reviews.apache.org/r/25071/#comment90154>

    Why make this change?  This utility method could be used other places, but marking it with self means that now an instance is required to use it.   Also, the call syntax is wrong in that now you need an implied self.get_host_from_url



ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java
<https://reviews.apache.org/r/25071/#comment90155>

    How is a registration alert definition command different from the other kind?


- Nate Cole


On Aug. 26, 2014, 8:17 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, 8:17 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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
>   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 
>   ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 
> 
> Diff: https://reviews.apache.org/r/25071/diff/
> 
> 
> Testing
> -------
> 
> Results :
> Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14
> 
> ** Note: Found a race condition issue with a test case that was causing inconsistent failures; this is what the change to ServiceComponentHostImpl/Test is **
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

> On Aug. 26, 2014, 9:20 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java, lines 97-99
> > <https://reviews.apache.org/r/25071/diff/2/?file=669884#file669884line97>
> >
> >     Can we get rid of the empty docs in this file?

A relic from the days when I used to always put @inheritDoc on methods; I still like the separation it gives to methods in the file (and javadoc inheritance still works) - but if it's not Ambari style, I'll remove them.


> On Aug. 26, 2014, 9:20 p.m., Tom Beerbower wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java, lines 165-172
> > <https://reviews.apache.org/r/25071/diff/2/?file=669884#file669884line165>
> >
> >     It looks like this doesn't really update anything.  Would it be better to throw an UnsupportedOperationException?

I'm starting work on both update methods today; I'll add the exception just to have the check-in in a correct state.


- Jonathan


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


On Aug. 26, 2014, 8:17 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, 8:17 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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
>   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 
>   ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 
> 
> Diff: https://reviews.apache.org/r/25071/diff/
> 
> 
> Testing
> -------
> 
> Results :
> Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14
> 
> ** Note: Found a race condition issue with a test case that was causing inconsistent failures; this is what the change to ServiceComponentHostImpl/Test is **
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

Ship it!


Looks good.  A couple of minor comments...


ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java
<https://reviews.apache.org/r/25071/#comment90107>

    Can we get rid of the empty docs in this file?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertGroupResourceProvider.java
<https://reviews.apache.org/r/25071/#comment90108>

    It looks like this doesn't really update anything.  Would it be better to throw an UnsupportedOperationException?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
<https://reviews.apache.org/r/25071/#comment90109>

    Can we remove the empty docs?



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProvider.java
<https://reviews.apache.org/r/25071/#comment90110>

    Update... same as above comment.


- Tom Beerbower


On Aug. 27, 2014, 12:17 a.m., Jonathan Hurley wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25071/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2014, 12:17 a.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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
>   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 
>   ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 
> 
> Diff: https://reviews.apache.org/r/25071/diff/
> 
> 
> Testing
> -------
> 
> Results :
> Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14
> 
> ** Note: Found a race condition issue with a test case that was causing inconsistent failures; this is what the change to ServiceComponentHostImpl/Test is **
> 
> 
> Thanks,
> 
> Jonathan Hurley
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

(Updated Aug. 26, 2014, 8:17 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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
  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 
  ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 

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


Testing (updated)
-------

Results :
Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14

** Note: Found a race condition issue with a test case that was causing inconsistent failures; this is what the change to ServiceComponentHostImpl/Test is **


Thanks,

Jonathan Hurley

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

(Updated Aug. 26, 2014, 8:15 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 (updated)
-----

  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/AlertTargetService.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/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostImpl.java cc5bb5b7fc5d05761f729b2f55f8343da1618d26 
  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 
  ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java c0bdaa29a9f8592c306e32b41835cb0f7d819d3c 

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


Testing
-------


Results :

Tests run: 1945, Failures: 0, Errors: 0, Skipped: 14


Thanks,

Jonathan Hurley

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

> On Aug. 26, 2014, 3: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?

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 ?


- Jonathan


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


On Aug. 26, 2014, 2: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, 2: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
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

> On Aug. 26, 2014, 3: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 ?
> 
> Tom Beerbower wrote:
>     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?
> 
> Jonathan Hurley wrote:
>     I'd say it would be a category to group targets and history (and potentially anything else that will fall under cross-cluster alerts). Requesting /v1/alerts could return the current alert states of all clusters, for example. It's not something that's in the specification since it's a post code complete concern, but we were trying to plan ahead for it.
>     
>     I see your point about it not fitting the model. I'll defer to you on v1/alerts vs v1/alert_targets & v1/alert_histories & v1/alerts_current. If you think it's best to follow the convention, I'll make the appropriate change.
> 
> Tom Beerbower wrote:
>     Ok, but I'm still open to discussion if what I suggested doesn't meet the requirements or somehow feels wrong.
>     
>     It seems to me like v1/alerts would return all of the root level (cross-cluster) alerts, if there is such a thing.  But then you should be able to ask for an individual root level alert with v1/alerts/{id}.  I don't know if that makes sense.  Also, if targets and histories are sub-resources of alerts then I would expect to be able to ask for all of the targets or histories of a specific alert... v1/alerts/{id}/targets.  Again, I don't know if that makes sense.  Should you be able to access a cross-cluster alert by id?  If so, then maybe the hierarchy makes sense.

I can see there being the need for an endpoint like v1/alerts that gets back all current alerts across clusters. This would provide an easy way to see cross-cluster alert state with a single call. However, you make a good point about what the sub-resources would look like. I don't think that v1/alerts/{id} makes sense.

My thought is to change the endpoint to be consistent for now and I can open up a discussion with others about how other types of data might fit in.


- Jonathan


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


On Aug. 26, 2014, 2: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, 2: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
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

Posted by Tom Beerbower <tb...@hortonworks.com>.

> 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 ?
> 
> Tom Beerbower wrote:
>     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?
> 
> Jonathan Hurley wrote:
>     I'd say it would be a category to group targets and history (and potentially anything else that will fall under cross-cluster alerts). Requesting /v1/alerts could return the current alert states of all clusters, for example. It's not something that's in the specification since it's a post code complete concern, but we were trying to plan ahead for it.
>     
>     I see your point about it not fitting the model. I'll defer to you on v1/alerts vs v1/alert_targets & v1/alert_histories & v1/alerts_current. If you think it's best to follow the convention, I'll make the appropriate change.

Ok, but I'm still open to discussion if what I suggested doesn't meet the requirements or somehow feels wrong.

It seems to me like v1/alerts would return all of the root level (cross-cluster) alerts, if there is such a thing.  But then you should be able to ask for an individual root level alert with v1/alerts/{id}.  I don't know if that makes sense.  Also, if targets and histories are sub-resources of alerts then I would expect to be able to ask for all of the targets or histories of a specific alert... v1/alerts/{id}/targets.  Again, I don't know if that makes sense.  Should you be able to access a cross-cluster alert by id?  If so, then maybe the hierarchy makes sense.


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

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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

> On Aug. 26, 2014, 3: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 ?
> 
> Tom Beerbower wrote:
>     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?

I'd say it would be a category to group targets and history (and potentially anything else that will fall under cross-cluster alerts). Requesting /v1/alerts could return the current alert states of all clusters, for example. It's not something that's in the specification since it's a post code complete concern, but we were trying to plan ahead for it.

I see your point about it not fitting the model. I'll defer to you on v1/alerts vs v1/alert_targets & v1/alert_histories & v1/alerts_current. If you think it's best to follow the convention, I'll make the appropriate change.


- Jonathan


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


On Aug. 26, 2014, 2: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, 2: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
> 
>

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

Posted by Tom Beerbower <tb...@hortonworks.com>.

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

Re: Review Request 25071: Alerts: Create Group and Target REST Endpoints

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



ambari-server/src/main/java/org/apache/ambari/server/api/services/AlertingService.java
<https://reviews.apache.org/r/25071/#comment90004>

    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?


- Tom Beerbower


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