> On July 22, 2015, 4:59 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java, > > line 307 > > <https://reviews.apache.org/r/36704/diff/1/?file=1019027#file1019027line307> > > > > This seems like it is not correct and that only items with null (or > > empty) hosts can be updated.
updateHostRoleState is public, but this call is only from ServerActionExecutor (and should only be null since we're server-side from this call). > On July 22, 2015, 4:59 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java, > > lines 157-161 > > <https://reviews.apache.org/r/36704/diff/1/?file=1019023#file1019023line157> > > > > Why not use the following, for consistency: > > > > ``` > > String hostName = safeHost(command.getHostName()); > > ``` Because I missed that one :) Will fix. > On July 22, 2015, 4:59 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java, > > line 434 > > <https://reviews.apache.org/r/36704/diff/1/?file=1019023#file1019023line434> > > > > Shouldn't `hostname` be removed? Or maybe we can use this to set some > > sort of host-affinity? I see your point - the method is only for server-side actions and should not be settable. Will fix. - Nate ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36704/#review92646 ----------------------------------------------------------- On July 22, 2015, 3:59 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, 3:59 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/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/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/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 > >
