> On July 21, 2015, 5:42 p.m., Yan Fang wrote: > > samza-core/src/main/java/org/apache/samza/metrics/MetricsBase.java, line 23 > > <https://reviews.apache.org/r/36473/diff/3/?file=1017435#file1017435line23> > > > > of the class "the" extends -> of the class "that" extends
Thanks > On July 21, 2015, 5:42 p.m., Yan Fang wrote: > > samza-core/src/main/java/org/apache/samza/metrics/MetricsBase.java, line 29 > > <https://reviews.apache.org/r/36473/diff/3/?file=1017435#file1017435line29> > > > > can we also have a constructor with the default prefix ""? Good idea > On July 21, 2015, 5:42 p.m., Yan Fang wrote: > > samza-core/src/main/scala/org/apache/samza/metrics/MetricsHelper.scala, > > lines 36-37 > > <https://reviews.apache.org/r/36473/diff/3/?file=1017436#file1017436line36> > > > > though it works, prefer to use the "def" here, not only because it has > > leff overhead, but also keep all the methods consistent for better > > readability. What do you think? Sounds good. I only baulked on it the first time because I'm not that skilled with Scala type decarations yet. :) I can make this work - Roger ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36473/#review92429 ----------------------------------------------------------- On July 21, 2015, 5:41 a.m., Roger Hoover wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36473/ > ----------------------------------------------------------- > > (Updated July 21, 2015, 5:41 a.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > SAMZA-733 Add metrics to Elasticsearch System Producer > > > Diffs > ----- > > samza-core/src/main/java/org/apache/samza/metrics/MetricGroup.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/metrics/MetricsBase.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/metrics/MetricsHelper.scala > 8eac8ef > > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemFactory.java > a277b69 > > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducer.java > 7eb14a2 > > samza-elasticsearch/src/main/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducerMetrics.java > PRE-CREATION > > samza-elasticsearch/src/test/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducerMetricsTest.java > PRE-CREATION > > samza-elasticsearch/src/test/java/org/apache/samza/system/elasticsearch/ElasticsearchSystemProducerTest.java > e63d62c > > Diff: https://reviews.apache.org/r/36473/diff/ > > > Testing > ------- > > Tested that metrics for Elasticsearch producer appear in JMX and the metrics > stream and that the metrics correctly count how many Elasticsearch documents > were created and indexed. > > > Thanks, > > Roger Hoover > >