----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56418/#review164728 -----------------------------------------------------------
Fix it, then Ship it! The code changes in this patch look fine to me, but I'd like to request two things: 1. Unit tests for this change be added to the patch 2. Some additional manual testing, mentioned below, to verify that the exported Blueprint will deploy properly. Thanks for providing this patch! ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java (line 75) <https://reviews.apache.org/r/56418/#comment236481> Overall, the changes look fine to me, but could you please add some unit tests for this change? If you could add some tests to the following: org.apache.ambari.server.api.query.render.ClusterBlueprintRendererTest that would be great. Thanks. ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java (line 76) <https://reviews.apache.org/r/56418/#comment236482> Could you also please try out some manual tests to use the exported Blueprint to create a new cluster? When changes to the export renderer are made, its always a good thing to try to make sure that the exported Blueprint is valid, and can be used for a successful cluster creation. Thanks. - Robert Nettleton On Feb. 8, 2017, 12:52 a.m., Madhuvanthi Radhakrishnan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56418/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2017, 12:52 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jayush Luniya, Robert > Nettleton, and Sumit Mohanty. > > > Bugs: AMBARI-19909 > https://issues.apache.org/jira/browse/AMBARI-19909 > > > Repository: ambari > > > Description > ------- > > Export Blueprints does not contain the settings object and hence the > credential store values > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java > 1a9ea91 > > Diff: https://reviews.apache.org/r/56418/diff/ > > > Testing > ------- > > Issued > http://c6401.ambari.apache.org:8080/api/v1/clusters/credential?format=blueprint > command to ensure that the settings object is present. > Resultant blueprint has the following object added: > "settings" : [ > { > "recovery_settings" : [ > { > "recovery_enabled" : "true" > } > ] > }, > { > "service_settings" : [ > { > "name" : "OOZIE", > "credential_store_enabled" : "true" > }, > { > "recovery_enabled" : "true", > "name" : "HIVE", > "credential_store_enabled" : "true" > }, > { > "recovery_enabled" : "true", > "name" : "HDFS" > }, > { > "recovery_enabled" : "true", > "name" : "AMBARI_METRICS" > } > ] > }, > { > "component_settings" : [ > { > "recovery_enabled" : "true", > "name" : "METRICS_COLLECTOR" > }, > { > "recovery_enabled" : "true", > "name" : "SECONDARY_NAMENODE" > }, > { > "recovery_enabled" : "true", > "name" : "WEBHCAT_SERVER" > } > ] > } > ] > > > Thanks, > > Madhuvanthi Radhakrishnan > >
