> On Sept. 30, 2014, 7:08 p.m., Nate Cole wrote: > > ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py, lines > > 320-321 > > <https://reviews.apache.org/r/26193/diff/1/?file=709770#file709770line320> > > > > No other path returns a value, and the caller of this method throws it > > away anyway. Can just return with no value.
Agreed; will fix. > On Sept. 30, 2014, 7:08 p.m., Nate Cole wrote: > > ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py, lines > > 340-341 > > <https://reviews.apache.org/r/26193/diff/1/?file=709770#file709770line340> > > > > Nate's Nit: log the str(exception) so we may have an idea of what went > > wrong. Also, if there is a try/except for each individual definition in > > the loop we could also identify which specific one failed. It looks like > > the only "bad" call could be the alert.collect() or the > > self.__json_to_callable() method. (stupid markdown won't let me do two > > underscores) The loop is stupid, but it's convention based on how the ActionQueue is setup for commands sent to the agents. 99.99% of the time, there will only ever be 1 command. But, I can change the logging to be more precise; I think that much of this method could cause problems, though. If the JSON is messed up, it could cause the agent to crash and restart (saw this during testing). logger.exception logs the message supplied and the caught exception; str(exception) doesn't always provide a full trace if I recall correctly. I will look into this as well before I push. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26193/#review55026 ----------------------------------------------------------- On Sept. 30, 2014, 6:38 p.m., Jonathan Hurley wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26193/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2014, 6:38 p.m.) > > > Review request for Ambari, Nate Cole and Tom Beerbower. > > > Bugs: AMBARI-7568 > https://issues.apache.org/jira/browse/AMBARI-7568 > > > Repository: ambari > > > Description > ------- > > An alert should be able to be scheduled for immediate execution. This is > independent of its interval scheduling. The on-demand execution should not > affect any existing job schedules on the agent for the definition. > > Alerts run outside of the realm of the normal request/task workflow. > Therefore, in order to trigger an on-demand alert to run, the following > command is used: > > POST > http://server/api/v1/clusters/{clusterName}/alert_definitions/{alertDefinitionId}?AlertDefinition/run_now=true > > The most sensible way to accomplish this was to create the new > AlertExecutionCommand. I know that we are conservative with our extension of > AgentCommand, however alerts falls into a new category since the agent is in > charge of all of the scheduling logic. It didn't make sense to make this a > custom command since there's no associated Python file to "run"; it's all > transmitted in the heartbeat and runs inside of the alerts framework. > > > Diffs > ----- > > ambari-agent/src/main/python/ambari_agent/AlertSchedulerHandler.py > c645cba0bb28c909894c9162f904c19f438c3304 > ambari-agent/src/main/python/ambari_agent/Controller.py > 18df06ed8c1960f670a9d4a5e9d1a7021330b9ea > ambari-agent/src/main/python/ambari_agent/alerts/collector.py > 9943211eab71d54598fbf58289b1e4171241aa9a > ambari-agent/src/test/python/ambari_agent/TestAlerts.py > f9c2ab4a93a2d56ae9d7f7d5051d3bc4a71bd1ae > > ambari-server/src/main/java/org/apache/ambari/server/agent/AgentCommand.java > 6e8aab1bf4236ffb129e12da4ff3c475338da299 > > ambari-server/src/main/java/org/apache/ambari/server/agent/AlertExecutionCommand.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatHandler.java > c8261539262ede385387919c7c28d9b827acccef > > ambari-server/src/main/java/org/apache/ambari/server/agent/HeartBeatResponse.java > 1e9dc125556cde334e5bb5ccd2c8c435335d3c89 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AlertDefinitionResourceProvider.java > e2d283728482ea301aff964a0e3e6ea3191b801c > > ambari-server/src/main/java/org/apache/ambari/server/state/alert/AlertDefinitionHash.java > 35d77426a9b0cf9373e95d29a48ccb8f1622e96b > ambari-server/src/main/resources/properties.json > 3893f39c0ce6a862bcbff53791748b6179238bc2 > > ambari-server/src/test/java/org/apache/ambari/server/state/alerts/AlertDefinitionHashTest.java > 1cb6eac7e57398e6eb0e18e45ed1ba5b4541efdf > > Diff: https://reviews.apache.org/r/26193/diff/ > > > Testing > ------- > > New tests added; ran the following project targets: > > ambari-agent$ mvn clean test > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 6.988 s > [INFO] Finished at: 2014-09-30T17:14:20-04:00 > [INFO] Final Memory: 8M/81M > [INFO] > ------------------------------------------------------------------------ > > ambari-server$ mvn clean test > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 18:51 min > [INFO] Finished at: 2014-09-30T18:02:03-04:00 > [INFO] Final Memory: 29M/180M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Jonathan Hurley > >
