> On May 12, 2015, 9:31 p.m., John Speidel wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java, > > line 1396 > > <https://reviews.apache.org/r/34123/diff/1/?file=956806#file956806line1396> > > > > what about double quoted style? > > Robert Nettleton wrote: > Double-quoted flow style was not necessary for this particular patch. > I've added the enum for the sake of completeness, but did not want to add > this unless we needed it for config processing. > > John Speidel wrote: > Seems dangerous to me to have the double quote flow available but not > implemented. I can certainly see somebody utilizing it in the future because > it is available via the enum and not checking to see if it is actually > implemented. If you don't want to implement it, it would be safer to remove > it from the enum.
It's important to note that this is an internal implementation class for the BlueprintConfigurationProcessor, and not a public-facing API. While I don't agree that this is dangerous, I'll remove the enum for double quoted flow style in order to resolve this issue. Once I remove the enum, I'll resubmit the patch. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34123/#review83489 ----------------------------------------------------------- On May 12, 2015, 8:55 p.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34123/ > ----------------------------------------------------------- > > (Updated May 12, 2015, 8:55 p.m.) > > > Review request for Ambari, John Speidel, Mahadev Konar, and Robert Levas. > > > Bugs: AMBARI-11087 > https://issues.apache.org/jira/browse/AMBARI-11087 > > > Repository: ambari > > > Description > ------- > > This patch resolves AMBARI-11087. > > The BlueprintConfigurationProcessor has been modified to account for a new > Storm property, "nimbus.seeds", that must be set to a comma-separated list of > Nimbus Server hosts. This property is used by Storm clients to discover the > list of HA Nimbus servers available in the cluster. This property uses Yaml, > and so the default Yaml PropertyUpdater in the > BlueprintConfigurationProcessor has been modified to account for different > Yaml Flow styles. This patch also registers this property updater to > "nimbus.seeds" for processing during cluster creation. > > This patch also adds unit tests to verify this change, and updates existing > unit tests to accomodate this change as well. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 7938cc1 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > e7b0c64 > > Diff: https://reviews.apache.org/r/34123/diff/ > > > Testing > ------- > > 1. Manual testing done with a 3-node cluster. Current problems with Storm > keep the initial Storm services from starting properly, but I was able to > verify that the "nimbus.seeds" configuration property is set correctly at the > completion of the cluster startup. With a local patch to Storm itself, I can > get all the Storm services to start properly. > 2. Ran the ambari-server unit tests, ran into 1 un-related failure: > "Failed tests: > testPopulateResourcesForRegexpMetrics(org.apache.ambari.server.controller.metrics.timeline.AMSPropertyProviderTest): > expected:<...rics.%25.AvailableMB[]&appId=RESOURCEMANAG...> but > was:<...rics.%25.AvailableMB[%2Cyarn.QueueMetrics.%25.AvailableMB._min%2Cyarn.QueueMetrics.%25.AvailableMB._max%2Cyarn.QueueMetrics.%25.AvailableMB._avg%2Cyarn.QueueMetrics.%25.AvailableMB._sum]&appId=RESOURCEMANAG…>". > This unit test failure will be fixed in a separate patch by the Metrics > team. > > > Thanks, > > Robert Nettleton > >
