----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50151/#review145963 -----------------------------------------------------------
Ship it! Overall looks good. Just jotted down some thoughts :) +1 build.gradle (line 513) <https://reviews.apache.org/r/50151/#comment212318> Curious: are we creating 2 tar tasks because we want all the bash scripts from samza-shell as a part of the rest tar ball? samza-rest/src/main/config/samza-rest.properties (line 23) <https://reviews.apache.org/r/50151/#comment212319> Wonder if there is a way to avoid this exact path and simply use an environment (system) variable. Just a thought. users need to configure it anyway. Or is it possible make "$PWD/.." to be deafult? Would it work ? samza-rest/src/main/java/org/apache/samza/monitor/MonitorLoader.java (line 23) <https://reviews.apache.org/r/50151/#comment212921> I wonder if it will be useful to have this as a more generic Utility class: ``` class ClassLoaderHelper<T> { ... public static T fromClassName(String klassName) { ... return (T) instance; } } ``` - Navina Ramesh On Aug. 17, 2016, 3:35 p.m., Jake Maes wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50151/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2016, 3:35 p.m.) > > > Review request for samza, Boris Shkolnik, Chris Pettitt, Jake Maes, Navina > Ramesh, Jagadish Venkatraman, Xinyu Liu, and Yi Pan (Data Infrastructure). > > > Bugs: SAMZA-975 > https://issues.apache.org/jira/browse/SAMZA-975 > > > Repository: samza > > > Description > ------- > > SAMZA-975 Initial Samza REST Implementation > > Implementation closely reflects the design from SAMZA-865. > In addition it also includes a basic Monitor feature, which enables users to > schedule arbitrary "monitor" logic to run periodically as part of the Samza > REST Service. > > > Diffs > ----- > > LICENSE e1439fe71052a97f1e5a2e189a645ce3e7422a47 > build.gradle 1d4eb74b1294318db8454631ddd0901596121ab2 > gradle/dependency-versions.gradle 47c71bfde027835682889407261d4798b629d214 > samza-rest/src/main/bash/run-samza-rest-service.sh PRE-CREATION > samza-rest/src/main/config/samza-rest.properties PRE-CREATION > samza-rest/src/main/java/org/apache/samza/monitor/Monitor.java PRE-CREATION > samza-rest/src/main/java/org/apache/samza/monitor/MonitorLoader.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/monitor/SamzaMonitorService.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/monitor/ScheduledExecutorSchedulingProvider.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/monitor/SchedulingProvider.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestApplication.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestConfig.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/SamzaRestService.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/model/Job.java PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/model/JobStatus.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/installation/InstallationFinder.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/installation/InstallationRecord.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/installation/SimpleInstallationFinder.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/AbstractJobProxy.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/JobInstance.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/JobProxy.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/JobProxyFactory.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/JobStatusProvider.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/ScriptJobProxy.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/SimpleYarnJobProxy.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/SimpleYarnJobProxyFactory.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/proxy/job/YarnCliJobStatusProvider.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/resources/DefaultResourceFactory.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResource.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/resources/JobsResourceConfig.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/resources/ResourceFactory.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/script/ScriptOutputHandler.java > PRE-CREATION > > samza-rest/src/main/java/org/apache/samza/rest/script/ScriptPathProvider.java > PRE-CREATION > samza-rest/src/main/java/org/apache/samza/rest/script/ScriptRunner.java > PRE-CREATION > samza-rest/src/main/resources/log4j.xml PRE-CREATION > samza-rest/src/test/java/org/apache/samza/monitor/TestMonitorService.java > PRE-CREATION > samza-rest/src/test/java/org/apache/samza/monitor/mock/DummyMonitor.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/monitor/mock/ExceptionThrowingMonitor.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/monitor/mock/InstantSchedulingProvider.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/rest/resources/TestJobsResource.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockJobProxy.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockJobProxyFactory.java > PRE-CREATION > > samza-rest/src/test/java/org/apache/samza/rest/resources/mock/MockJobStatusProvider.java > PRE-CREATION > samza-shell/src/main/bash/kill-yarn-job-by-name.sh PRE-CREATION > settings.gradle 4c1aa107a11d413777e69bc4e48847b811aff7d2 > > Diff: https://reviews.apache.org/r/50151/diff/ > > > Testing > ------- > > Unit tests pass. > Deployed on a local cluster and verified the hello-samza jobs are listed at > localhost:9139/v1/jobs > The service has been deployed for months in LinkedIn with additional > Resources and Monitors. > Also ran bin/check-all.sh > > > Thanks, > > Jake Maes > >
