-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51703/#review150571
-----------------------------------------------------------




samza-rest/src/main/java/org/apache/samza/monitor/MonitorConfig.java (line 55)
<https://reviews.apache.org/r/51703/#comment218586>

    It would be good to split this method into two parts for modularity and 
readability. 
    
    1. List<String> getMonitorNames() which is very similar to 
SystemConfig.getSystemNames()
    2. Map<String, MonitorConfig> getMonitorConfigs(config, monitorNames)



samza-rest/src/main/java/org/apache/samza/monitor/MonitorConfig.java (line 62)
<https://reviews.apache.org/r/51703/#comment218583>

    putIfAbsent doesn't exist in Java 7
    
    A simple put() should suffice. 
    
    If you want to make sure the same key isn't added twice, you could assert 
that the return value of put() is null. I think that case should be an error 
because the way the current code supports it, the behavior is not well-defined.


- Jake Maes


On Sept. 26, 2016, 11:14 p.m., Shanthoosh Venkataraman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51703/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2016, 11:14 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.
> JIRA ticket : https://issues.apache.org/jira/browse/SAMZA-1024
> 
> 
> 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
> 
>

Reply via email to