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