----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36704/#review92691 -----------------------------------------------------------
Ship it! ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java (lines 289 - 299) <https://reviews.apache.org/r/36704/#comment146919> Instead of duplicating this code, could you instead just do something like ``` String outputPrefix = "output-"; String errorsPrefix = "errors-" if(null != prefix){ outputPrefix = prefix + outputPrefix; errrosPrefix = prefix + errorsPrefix; } ``` ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java (line 138) <https://reviews.apache.org/r/36704/#comment146923> thank you! ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java (line 769) <https://reviews.apache.org/r/36704/#comment146924> It makes me nervous that host can be null now; feels too easy to hit a NPE. Should we at least doc that this could be null? Any way to fake it and make it not null? Have a dummy host impl with the name of Ambari Server? Most of its methods are NOOPs? ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java (line 738) <https://reviews.apache.org/r/36704/#comment146926> Rename to getSafeHost(...) ? It is a getter, right? :) ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java (lines 243 - 248) <https://reviews.apache.org/r/36704/#comment146928> Convert to named query. ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog220.java (line 34) <https://reviews.apache.org/r/36704/#comment146929> 2.2.0 - Jonathan Hurley On July 22, 2015, 7:26 p.m., Nate Cole wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36704/ > ----------------------------------------------------------- > > (Updated July 22, 2015, 7:26 p.m.) > > > Review request for Ambari, Jonathan Hurley and Robert Levas. > > > Bugs: AMBARI-12506 > https://issues.apache.org/jira/browse/AMBARI-12506 > > > Repository: ambari > > > Description > ------- > > Change the host_role_command table to allow null for host_id, indicating it's > running on the server. Changed classes to support that scenario. > > Chose this over adding a row to the host table because: > * That table is really used for hosts that have agents > * Makes it hard to support Ambari-failover, Ambari server host changes > (including from-backup scenarios) > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 51b2f09 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java > 562a5ca > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/HostRoleCommand.java > 85e9135 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java > 17411ac > > ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelperImpl.java > 4ae1260 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java > 2696cca > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > afa3c08 > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostRoleCommandEntity.java > a2e5b31 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java > a702599 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java > 5dc58e8 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog220.java > PRE-CREATION > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 700f9bf > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 209e8f8 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql fa11804 > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > ae4eef0 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 6ede40f > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/StageTest.java > bb64e48 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java > c8e7dbd > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java > b0edc3e > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/UpgradeResourceProviderTest.java > 69f0be7 > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java > 62b2fdf > > ambari-server/src/test/java/org/apache/ambari/server/stageplanner/TestStagePlanner.java > 3f856bd > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog220Test.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/36704/diff/ > > > Testing > ------- > > Manual: Clean install; RU that has server-side; enable kerberos (one issue, > filed separately for UI). > Automated: > > Tests run: 3124, Failures: 0, Errors: 0, Skipped: 28 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 27:12 min > [INFO] Finished at: 2015-07-22T15:58:42-04:00 > [INFO] Final Memory: 33M/1341M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Nate Cole > >
