Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 18, 2016, 10:18 p.m.) Review request for samza and Jake Maes. R

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review156078 --- Ship it! I'll try commiting it once the patch is posted on a JI

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 16, 2016, 5:44 a.m.) Review request for samza and Jake Maes. Re

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155753 --- Ship it! Ship It! - Fred Ji On Nov. 11, 2016, 12:22 a.m., Sh

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 11:18 p.m., Prateek Maheshwari wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55 > > > > > > What's the execution unit here? The samza-rest server? The monitor?

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155733 --- Fix it, then Ship it! After addressing the nit. Thanks! docs/

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155689 --- Ship it! Ship It! - Prateek Maheshwari On Nov. 10, 2016, 4:2

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 10, 2016, 10:57 a.m., Prateek Maheshwari wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line > > 103 > > > > > > Don't understand this interface. What's a SchedulingProvid

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 11, 2016, 12:22 a.m.) Review request for samza and Jake Maes. R

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 5, 2016, 6:47 p.m., Jake Maes wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line > > 75 > > > > > > I don't think the MetricsConfig constructure takes a subset. > >

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 10, 2016, 6:57 p.m., Prateek Maheshwari wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 27 > > > > > > Unused import. Removed. > On Nov. 10, 2016, 6:57 p.m., Prateek M

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155625 --- Looks pretty good to me. Some final minor comments and questions.

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 11:18 p.m., Prateek Maheshwari wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55 > > > > > > What's the execution unit here? The samza-rest server? The monitor?

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 12:09 a.m., Jagadish Venkatraman wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55 > > > > > > How exactly is this used when metrics are reported? > > > >

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 10, 2016, 1:04 a.m.) Review request for samza and Jake Maes. Re

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 10, 2016, 12:04 a.m.) Review request for samza and Jake Maes. R

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 11:18 p.m., Prateek Maheshwari wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55 > > > > > > What's the execution unit here? The samza-rest server? The monitor?

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 12:09 a.m., Jagadish Venkatraman wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55 > > > > > > How exactly is this used when metrics are reported? > > > >

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 5, 2016, 11:47 a.m., Jake Maes wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line > > 75 > > > > > > I don't think the MetricsConfig constructure takes a subset. > >

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155511 --- samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.ja

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 12:09 a.m., Jagadish Venkatraman wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java, line 55 > > > > > > How exactly is this used when metrics are reported? > > > >

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 12:09 a.m., Jagadish Venkatraman wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line > > 65 > > > > > > Why is this taking in a concrete class - ReadableMetricsReg

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 9, 2016, 10:50 p.m.) Review request for samza and Jake Maes. Re

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 9, 2016, 12:31 a.m., Prateek Maheshwari wrote: > > samza-core/src/main/java/org/apache/samza/util/MetricsReporterLoader.java, > > line 36 > > > > > > Minor: private constructors for helper classes are prett

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155380 --- docs/learn/documentation/versioned/rest/monitors.md (line 100)

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155372 --- docs/learn/documentation/versioned/rest/monitors.md (line 95)

Re: Review Request 53297: Initial version of adding metrics into samza rest.

- Shanthoosh --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155061 --- On Nov. 8, 2016, 11:13 p.m., Shanthoosh Venkatarama

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- (Updated Nov. 8, 2016, 11:13 p.m.) Review request for samza and Jake Maes. Re

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 5, 2016, 6:47 p.m., Jake Maes wrote: > > samza-core/src/main/scala/org/apache/samza/util/Util.scala, line 336 > > > > > > This edit looks like a mistake. > > > > Did this file need to be modified a

Re: Review Request 53297: Initial version of adding metrics into samza rest.

> On Nov. 5, 2016, 6:47 p.m., Jake Maes wrote: > > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java, line > > 75 > > > > > > I don't think the MetricsConfig constructure takes a subset. > >

Re: Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/#review155061 --- samza-core/src/main/scala/org/apache/samza/util/MetricsReporterLo

Review Request 53297: Initial version of adding metrics into samza rest.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53297/ --- Review request for samza. Repository: samza Description --- This patch a