> On Sept. 23, 2016, 8:33 p.m., Jake Maes wrote: > > samza-rest/src/main/java/org/apache/samza/monitor/MonitorLoader.java, line > > 34 > > <https://reviews.apache.org/r/51703/diff/2/?file=1498673#file1498673line34> > > > > Is it reasonable for this method to throw anything. e.g. > > InstantiationException? > > > > A unit test should show that if an exception occurs, the service > > handles it properly. > > > > Specifically the samza rest service should not fail because a new > > monitor is broken. It should definitely log it though.
Added throws Exception to method signature. Failures that occurred in any one of the samza monitors instantiation would fail the samza service startup. > On Sept. 23, 2016, 8:33 p.m., Jake Maes wrote: > > samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java, > > lines 75-76 > > <https://reviews.apache.org/r/51703/diff/2/?file=1498674#file1498674line75> > > > > What purpose does this serve? Samza monitor instantiation that uses reflection to create the monitor instances from factory, can throw InstantiationException. This catch block handles that. - Shanthoosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51703/#review150206 ----------------------------------------------------------- On Sept. 14, 2016, 8:37 p.m., Shanthoosh Venkataraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51703/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2016, 8:37 p.m.) > > > Review request for samza, Boris Shkolnik, Jake Maes, Navina Ramesh, Jagadish > Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure). > > > Repository: samza > > > Description > ------- > > This patch aims at adding the following functionalities to Samza-Rest > monitors. > > * Schedule different monitors at different intervals of time. > * Define custom monitor configurations and pass config along to the monitor > objects. > > > Diffs > ----- > > docs/learn/documentation/versioned/rest/monitors.md > 9833068d9f542b80bb438db156a10c85d4d53097 > docs/learn/documentation/versioned/rest/overview.md > 5b958accf4e1a3f05b949e9dc6e2e19a453523ca > samza-rest/src/main/java/org/apache/samza/monitor/AbstractMonitor.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/monitor/Monitor.java > d69df5f73dbf2c494183f9dcc8061c20878742aa > samza-rest/src/main/java/org/apache/samza/monitor/MonitorLoader.java > 502ecc49f32b4563e8ceb4ddfa6bc2542c60819e > samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java > 2f4d9ddb76369c5e83d39152d492807dfb164981 > samza-rest/src/main/java/org/apache/samza/monitor/SchedulingProvider.java > aea1a9291e651660c798cabf59fcf0c0623bcbd0 > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java > 6f5c10ac89523626c7f7e05558422daad2ccd4e8 > samza-rest/src/test/java/org/apache/samza/monitor/TestMonitorService.java > 1da343012b85f96f837e3cbf9a54ced3b29fede6 > samza-rest/src/test/java/org/apache/samza/monitor/mock/DummyMonitor.java > 8621db1b0e8ce3279cc8a5cb3a21bd137d442034 > > samza-rest/src/test/java/org/apache/samza/monitor/mock/ExceptionThrowingMonitor.java > c4f3f735f78d56f8bb3ef203a05e2bec92489767 > > Diff: https://reviews.apache.org/r/51703/diff/ > > > Testing > ------- > > Unit tests are used to verify the intended functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
