> On July 1, 2016, 3:54 p.m., Sid Wagle wrote: > > ambari-metrics/ambari-metrics-storm-sink-legacy/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSink.java, > > line 194 > > <https://reviews.apache.org/r/49514/diff/1/?file=1434988#file1434988line194> > > > > Good catch, however, why not look for "._" and replace that only if > > needed. > > Dmytro Sen wrote: > There is no change here. It's a copy of previous > StormTimelineMetricsSink.java
It's originally for escaping '._' but only escaping that case is not intuitive for users. Escaping all _ will make users to predict escaped metric name easily. > On July 1, 2016, 3:54 p.m., Sid Wagle wrote: > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/storm-site.xml, > > line 30 > > <https://reviews.apache.org/r/49514/diff/1/?file=1434997#file1434997line30> > > > > So both old and new properties are ok to be present at the same time? > > Dmytro Sen wrote: > yes yes since storm runs properly with unknown configs are presented. - Jungtaek ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49514/#review140370 ----------------------------------------------------------- On July 1, 2016, 3:32 p.m., Dmytro Sen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49514/ > ----------------------------------------------------------- > > (Updated July 1, 2016, 3:32 p.m.) > > > Review request for Ambari, Aravindan Vijayan, Jungtaek Lim, and Sid Wagle. > > > Bugs: AMBARI-17080 > https://issues.apache.org/jira/browse/AMBARI-17080 > > > Repository: ambari > > > Description > ------- > > Support 2 storm sinks. > > > Diffs > ----- > > ambari-metrics/ambari-metrics-assembly/pom.xml 5e2d819 > ambari-metrics/ambari-metrics-assembly/src/main/assembly/sink-windows.xml > e82d2d4 > ambari-metrics/ambari-metrics-assembly/src/main/assembly/sink.xml 4a3b7c5 > ambari-metrics/ambari-metrics-storm-sink-legacy/pom.xml PRE-CREATION > > ambari-metrics/ambari-metrics-storm-sink-legacy/src/main/assemblies/empty.xml > PRE-CREATION > > ambari-metrics/ambari-metrics-storm-sink-legacy/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsReporter.java > PRE-CREATION > > ambari-metrics/ambari-metrics-storm-sink-legacy/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSink.java > PRE-CREATION > > ambari-metrics/ambari-metrics-storm-sink-legacy/src/test/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSinkTest.java > PRE-CREATION > ambari-metrics/ambari-metrics-storm-sink/pom.xml 1591d39 > > ambari-metrics/ambari-metrics-storm-sink/src/main/conf/storm-metrics2.properties.j2 > 4553224 > > ambari-metrics/ambari-metrics-storm-sink/src/main/java/org/apache/hadoop/metrics2/sink/storm/NumberUtil.java > PRE-CREATION > > ambari-metrics/ambari-metrics-storm-sink/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsReporter.java > ab5f1e4 > > ambari-metrics/ambari-metrics-storm-sink/src/main/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSink.java > 6ab12e1 > > ambari-metrics/ambari-metrics-storm-sink/src/test/java/org/apache/hadoop/metrics2/sink/storm/StormTimelineMetricsSinkTest.java > e582a95 > ambari-metrics/pom.xml 7221ab5 > > ambari-server/src/main/resources/common-services/AMBARI_METRICS/0.1.0/configuration/storm-site.xml > 0d029e8 > > ambari-server/src/main/resources/common-services/STORM/0.9.1/package/scripts/params_linux.py > 073bb1c > > ambari-server/src/main/resources/common-services/STORM/0.9.1/package/scripts/storm.py > 2d50767 > > ambari-server/src/main/resources/common-services/STORM/0.9.1/package/scripts/ui_server.py > 6551067 > > ambari-server/src/main/resources/common-services/STORM/0.9.1/package/templates/storm-metrics2.properties.j2 > 9acf173 > > ambari-server/src/main/resources/stacks/HDP/2.5/services/STORM/configuration/storm-site.xml > f3bbce8 > > Diff: https://reviews.apache.org/r/49514/diff/ > > > Testing > ------- > > Unit testst passed > > > Thanks, > > Dmytro Sen > >
