----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27700/#review60796 -----------------------------------------------------------
Overall, looks good. ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java <https://reviews.apache.org/r/27700/#comment102207> Use braces for all if statements. Not sure if this is explicitly called out in our coding standards but not using braces is generally frowned upon as it is error prone. ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java <https://reviews.apache.org/r/27700/#comment102210> javadoc Seems odd that only the setters are in the interface ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java <https://reviews.apache.org/r/27700/#comment102211> javadoc ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java <https://reviews.apache.org/r/27700/#comment102212> typo is -> it's ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java <https://reviews.apache.org/r/27700/#comment102213> javadoc for members ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java <https://reviews.apache.org/r/27700/#comment102232> Should consider using an executor instead of directly creating threads here. This will simplify and provide more flexibility if we choose to change the concurrency strategy for server tasks. Generally using an executor is preferred to rolling your own unless you have a really good reason to do otherwise. ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java <https://reviews.apache.org/r/27700/#comment102228> if join() is interrupted, do we want to propagate the interrupt to the worker thread? ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java <https://reviews.apache.org/r/27700/#comment102233> class is missing all javadoc's - John Speidel On Nov. 10, 2014, 2:05 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27700/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 2:05 p.m.) > > > Review request for Ambari, dilli dorai, Jonathan Hurley, John Speidel, Nate > Cole, Robert Nettleton, and Sid Wagle. > > > Bugs: AMBARI-7985 > https://issues.apache.org/jira/browse/AMBARI-7985 > > > Repository: ambari > > > Description > ------- > > Ambari currently handles client-/agent-side commands; however there is no > ability to handle server-side commands. Server-side commands should be > specified as a task in a stage and managed along with the stage. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessor.java > 1f99b4a > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 5e879cc > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionManager.java > e2fad5f > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java > 81fee75 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java > bbc5ac3 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > 52b0ba6 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > 6920a9e > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerAction.java > be885b5 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManager.java > 011cf06 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManagerImpl.java > 3a16c77 > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEvent.java > 78590fc > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEventType.java > b43ac9c > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostUpgradeEvent.java > 8b375fe > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java > 36acbc2 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java > 5a2c467 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java > 7224924 > > ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java > 6e78b1d > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > b2c023f > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java > PRE-CREATION > > ambari-server/src/test/python/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/27700/diff/ > > > Testing > ------- > > ------------------------------------------------------------------------------- > Test set: org.apache.ambari.server.serveraction.ServerActionExecutorTest > ------------------------------------------------------------------------------- > Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.822 sec > > > Thanks, > > Robert Levas > >
