> On April 23, 2015, 6:47 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/cache/ConfigGroupHostMappingImpl.java,
> >  line 38
> > <https://reviews.apache.org/r/33464/diff/1/?file=940149#file940149line38>
> >
> >     Defer to getHost().getId()?  Why have another function just for that

That would have been ideal. However, getHost() returns a Host object as opposed 
to a HostEntity, which does contain the hostId. This class does not contain a 
reference to HostEntity.


> On April 23, 2015, 6:47 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/orm/cache/ConfigGroupHostMappingImpl.java,
> >  lines 101-102
> > <https://reviews.apache.org/r/33464/diff/1/?file=940149#file940149line101>
> >
> >     If hosts are not-equal and the ids are not-equal, then there's a bigger 
> > problem :)

Host interface doesn't require an equals implementation, and HostImpl doesn't 
override the equals method, so I'll create one where it compares the hostName.


> On April 23, 2015, 6:47 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java,
> >  lines 979-984
> > <https://reviews.apache.org/r/33464/diff/1/?file=940147#file940147line979>
> >
> >     Still a hack, and if the hostsId is null then a very ugly exception is 
> > going to happen

I'll make it more robust.


- Alejandro


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33464/#review81367
-----------------------------------------------------------


On April 23, 2015, 2:11 a.m., Alejandro Fernandez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33464/
> -----------------------------------------------------------
> 
> (Updated April 23, 2015, 2:11 a.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley, Nate Cole, Sumit Mohanty, and Sid 
> Wagle.
> 
> 
> Bugs: AMBARI-10679
>     https://issues.apache.org/jira/browse/AMBARI-10679
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Refactor serviceconfighosts, hostconfigmapping , and configgrouphostmapping 
> to use host_id instead of host_name
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProvider.java
>  3fcb84b 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
>  82b7307 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/cache/ConfigGroupHostMapping.java
>  5c26a6c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/cache/ConfigGroupHostMappingImpl.java
>  54e1ca0 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/cache/HostConfigMapping.java
>  269daa9 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/cache/HostConfigMappingImpl.java
>  407aeb6 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/ConfigGroupHostMappingDAO.java
>  592679e 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostConfigMappingDAO.java
>  9bc1235 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostDAO.java 
> 6442bf5 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ConfigGroupHostMappingEntity.java
>  261bbe8 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ConfigGroupHostMappingEntityPK.java
>  3ee2b6c 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostConfigMappingEntity.java
>  1411a67 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostConfigMappingEntityPK.java
>  16111fb 
>   
> ambari-server/src/main/java/org/apache/ambari/server/orm/entities/ServiceConfigEntity.java
>  1a31252 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Cluster.java 
> 847e349 
>   ambari-server/src/main/java/org/apache/ambari/server/state/Clusters.java 
> 80ac6a7 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
>  6055eb8 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClustersImpl.java
>  70788ff 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroup.java
>  a4cc6ac 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupFactory.java
>  d4597af 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/configgroup/ConfigGroupImpl.java
>  ffa085a 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/host/HostImpl.java 
> 41cfee7 
>   
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog210.java
>  0373aac 
>   ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql b6f2aaa 
>   ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql 25685e5 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 9ade56f 
>   ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql 
> feaeae9 
>   ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 03f1ec8 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java
>  06f9e8a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ConfigGroupResourceProviderTest.java
>  db324e5 
>   
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/ConfigGroupDAOTest.java
>  2adbf9d 
>   
> ambari-server/src/test/java/org/apache/ambari/server/orm/dao/HostConfigMappingDAOTest.java
>  ec1289a 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/ConfigGroupTest.java
>  28059c0 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java
>  930e45f 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java
>  e076d4e 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostTest.java
>  fde1945 
>   
> ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeTest.java 
> ddfab75 
> 
> Diff: https://reviews.apache.org/r/33464/diff/
> 
> 
> Testing
> -------
> 
> Local unit tests passed, except for an unrelated failure in 
> KerberosServiceMetaInfoTest
> Applied DB changes to a cluster, and was able to create config groups, 
> override properties, etc.
> 
> SQL commands:
> 
> ALTER TABLE serviceconfighosts DROP CONSTRAINT serviceconfighosts_pkey;
> ALTER TABLE serviceconfighosts ADD COLUMN host_id BIGINT NOT NULL;
> ALTER TABLE serviceconfighosts ADD CONSTRAINT serviceconfighosts_pkey PRIMARY 
> KEY (service_config_id, host_id);
> ALTER TABLE serviceconfighosts ADD CONSTRAINT FK_scvhosts_host_id FOREIGN KEY 
> (host_id) REFERENCES hosts (host_id);
> ALTER TABLE serviceconfighosts DROP COLUMN hostname;
> 
> 
> ALTER TABLE hostconfigmapping DROP CONSTRAINT hostconfigmapping_pkey;
> ALTER TABLE hostconfigmapping DROP CONSTRAINT fk_hostconfmapping_host_name;
> ALTER TABLE hostconfigmapping ADD COLUMN host_id BIGINT NOT NULL;
> ALTER TABLE hostconfigmapping ADD CONSTRAINT hostconfigmapping_pkey PRIMARY 
> KEY (cluster_id, host_id, type_name, create_timestamp);
> ALTER TABLE hostconfigmapping ADD CONSTRAINT FK_hostconfmapping_host_id 
> FOREIGN KEY (host_id) REFERENCES hosts (host_id);
> ALTER TABLE hostconfigmapping DROP COLUMN host_name;
> 
> ALTER TABLE configgrouphostmapping DROP CONSTRAINT 
> configgrouphostmapping_pkey;
> ALTER TABLE configgrouphostmapping DROP CONSTRAINT 
> fk_hostconfmapping_host_name;
> ALTER TABLE configgrouphostmapping ADD COLUMN host_id BIGINT NOT NULL;
> ALTER TABLE configgrouphostmapping ADD CONSTRAINT configgrouphostmapping_pkey 
> PRIMARY KEY (config_group_id, host_id);
> ALTER TABLE configgrouphostmapping ADD CONSTRAINT FK_cghm_host_id FOREIGN KEY 
> (host_id) REFERENCES hosts (host_id);
> ALTER TABLE configgrouphostmapping DROP COLUMN host_name;
> 
> 
> Thanks,
> 
> Alejandro Fernandez
> 
>

Reply via email to