> On Sept. 3, 2015, 12:58 a.m., John Speidel wrote: > > Overall, good job on the patch and I am happy that you have tests for your > > changes. > > Couple of comments/questions: > > - Why not remove cluster name from the table and just have a required > > cluster ID? I wouldn't expect that we would want to continue to persist > > the name. > > - For cases where a BP deployed cluster has already been renamed, it is > > impossible to recover the cluster ID so this patch only fixes things if it > > is applied prior to a rename occurring (unless I missed something). As > > Ambari currently only can manage a single cluster, perhaps we default to > > the existing cluster if we can't match the name to an ID? > > - It might be a good idea to remove all usages of cluster name in the > > topology code since we really only care about the ID. If we really need > > the name, for a log msg for example, we can get the name using the ID.
Actually, in the case where a cluster rename has already occurred prior to upgrade, the cluster name from the topology request won't match any current cluster name. In this case since Ambari can currently manage only one cluster, we could just set the id of the only cluster known to ambari. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38001/#review97576 ----------------------------------------------------------- On Sept. 1, 2015, 11:53 a.m., Andrew Onischuk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38001/ > ----------------------------------------------------------- > > (Updated Sept. 1, 2015, 11:53 a.m.) > > > Review request for Ambari, John Speidel and Robert Nettleton. > > > Bugs: AMBARI-12538 > https://issues.apache.org/jira/browse/AMBARI-12538 > > > Repository: ambari > > > Description > ------- > > Attempting to access the Ambari Dashboard results in an HTTP 500 Error after > changing cluster name and restarting the Ambari server > > Refer to <https://issues.apache.org/jira/browse/AMBARI-12538> for more info. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java > 8b13826 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java > 0150141 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ScaleClusterRequest.java > a6995ed > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/TopologyRequestEntity.java > 8535ed8 > > ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java > 40717cc > > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java > 9a9929b > > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java > 9dbd197 > > ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java > cc82a63 > > ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java > b7f95cf > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedState.java > dbf6735 > > ambari-server/src/main/java/org/apache/ambari/server/topology/PersistedStateImpl.java > 894bc6d > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyManager.java > 5f3bb9d > > ambari-server/src/main/java/org/apache/ambari/server/topology/TopologyRequest.java > d102c21 > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog212.java > 02df181 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 9828eeb > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 22f7493 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 9fc1239 > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 0c966ca > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql 9f21160 > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 58760ff > > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterTopologyImplTest.java > bbea96c > > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java > 64dfb28 > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog212Test.java > 7a394ca > > Diff: https://reviews.apache.org/r/38001/diff/ > > > Testing > ------- > > mvn clean test > > > Thanks, > > Andrew Onischuk > >
