----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62809/#review187418 -----------------------------------------------------------
Ship it! Overall, the patch looks fine to me, just a comment below about making sure this is tested thoroughly, since this patch does introduce a slight change to the Blueprints model for updater registration. Thanks. ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java Lines 2812 (patched) <https://reviews.apache.org/r/62809/#comment264369> Generally, the model here is to create a new property updater for each property. Off the top of my head, it seems fine to use a singleton for the property updater, but please make sure this change is tested thoroughly, since there might be un-intended consequences of changing the way these updaters are registered. - Robert Nettleton On Oct. 9, 2017, 12:43 p.m., Attila Doroszlai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62809/ > ----------------------------------------------------------- > > (Updated Oct. 9, 2017, 12:43 p.m.) > > > Review request for Ambari, Nishant Bangarwa, Slim Bouguerra, Sandor Magyari, > Swapan Shridhar, and Sebastian Toader. > > > Bugs: AMBARI-22159 > https://issues.apache.org/jira/browse/AMBARI-22159 > > > Repository: ambari > > > Description > ------- > > Replace placeholders in `metastore_hostname`, > `druid.metadata.storage.connector.connectURI` and `druid.zk.service.host` > when creating cluster via blueprint. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 5a6e2cc0de7dc0c0f4499624e21c43bbc8d24b54 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > 68d6349db0207ab6df44b4b0a341c77ecabc46ea > > > Diff: https://reviews.apache.org/r/62809/diff/2/ > > > Testing > ------- > > Tested manually on local cluster. > > Unit tests pass: `Tests run: 4900, Failures: 0, Errors: 0, Skipped: 36` > > > Thanks, > > Attila Doroszlai > >
