> On Sept. 26, 2016, 7:37 p.m., Jake Maes wrote: > > samza-rest/src/test/java/org/apache/samza/monitor/TestMonitorService.java, > > lines 81-84 > > <https://reviews.apache.org/r/51703/diff/4/?file=1509611#file1509611line81> > > > > If I'm reading this correctly, this test depends on MapConfig to retain > > order of the configs, and that may not hold true.
Order of the config does not matter here, since there are no common keys betwen two config maps. Combining two config maps into one config object, would not cause problem due to uniqueness of all the keys. > On Sept. 26, 2016, 7:37 p.m., Jake Maes wrote: > > samza-rest/src/test/java/org/apache/samza/monitor/TestMonitorService.java, > > lines 63-68 > > <https://reviews.apache.org/r/51703/diff/4/?file=1509611#file1509611line63> > > > > I think it's importand to have some kind of follow-up test after > > instantiation to make sure the monitor was actually instantiated and > > returned, rather than null, for example. > > > > Minimum requirements would be > > 1. not null > > 2. instance of Monitor Instance of Monitor is always true, since the factory returns Monitor object back. Added not null & monitor checks. - Shanthoosh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51703/#review150435 ----------------------------------------------------------- On Sept. 24, 2016, 12:34 a.m., Shanthoosh Venkataraman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51703/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2016, 12:34 a.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/Monitor.java > d69df5f73dbf2c494183f9dcc8061c20878742aa > samza-rest/src/main/java/org/apache/samza/monitor/MonitorConfig.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/monitor/MonitorFactory.java > PRE-CREATION > 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/main/java/org/apache/samza/rest/SamzaRestService.java > 5b34da814985fb09281f36c677a97372cacc7a75 > 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/DummyMonitorFactory.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/monitor/mock/ExceptionThrowingMonitor.java > c4f3f735f78d56f8bb3ef203a05e2bec92489767 > > samza-rest/src/test/java/org/apache/samza/monitor/mock/ExceptionThrowingMonitorFactory.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51703/diff/ > > > Testing > ------- > > Unit tests are used to verify the intended functionality. > > > Thanks, > > Shanthoosh Venkataraman > >
