----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53002/#review153224 -----------------------------------------------------------
Fix it, then Ship it! One minor issue below. Once that's fixed and the patch is attached to the JIRA, I'll commit it. samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java (line 63) <https://reviews.apache.org/r/53002/#comment222509> Commons is not an explicit dependency of samza-rest in the build.gradle file, but Guava is. Can this be done with Strings? https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Strings.html Commons is probably a transitive dependency. So while, we could get away with it now, it could break later. - Jake Maes On Oct. 18, 2016, 10:53 p.m., Shanthoosh Venkataraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53002/ > ----------------------------------------------------------- > > (Updated Oct. 18, 2016, 10:53 p.m.) > > > Review request for samza and Jake Maes. > > > Repository: samza > > > Description > ------- > > This patch aims to not load the monitors if the monitor factory class is not > defined(set) for the monitor in the config. This will enable the users to > turn on/off the monitors in samza-rest easily(just by setting the > monitorFactoryClass config associated with monitor a to empty string.) > > > Diffs > ----- > > samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java > ce947f7ae1175acc1ee9aa75991c726848072694 > samza-rest/src/test/java/org/apache/samza/monitor/TestMonitorService.java > 4618b54f5af861383df45bf7185622d36d17cd5e > > samza-rest/src/test/java/org/apache/samza/monitor/mock/MockMonitorFactory.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/53002/diff/ > > > Testing > ------- > > Unit testing and manual testing are done to verify the functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
