----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40638/#review107921 -----------------------------------------------------------
lens-api/src/main/java/org/apache/lens/api/util/DateUtil.java (line 32) <https://reviews.apache.org/r/40638/#comment167254> Can you add comments on what each method is for here? Since this goes in lens-api, i'm thinking you are planning to use all these on client side. If validation happens server side, we should move the class to lens-server-api lens-api/src/main/java/org/apache/lens/api/util/DateUtil.java (line 56) <https://reviews.apache.org/r/40638/#comment167255> Instead of RuntimeException, can they defined as LensException or LensAPIException with error codes added? lens-api/src/main/resources/example-job.xml (line 43) <https://reviews.apache.org/r/40638/#comment167256> start_time says it is starting from 2014, but cron says run in 2005. We should change this to be a different year - something between start and end lens-api/src/main/resources/job-0.1.xsd (line 1) <https://reviews.apache.org/r/40638/#comment167265> file name as job-0.1.xsd seems very generic. We should have it as schedule-0.1.xsd or schedule-job-0.1.xsd lens-api/src/main/resources/job-0.1.xsd (line 35) <https://reviews.apache.org/r/40638/#comment167267> Please add <xs:annotation><xs:documentation> for all elements and attributes. See cube-0.1.xsd, for example. lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/Scheduler.java (line 32) <https://reviews.apache.org/r/40638/#comment167268> I would say lets have class name as SchedulerService for interface and SchedulerServiceImpl for implementation to be consistent with other classes. lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/Scheduler.java (line 67) <https://reviews.apache.org/r/40638/#comment167271> Please explain return value for all methods lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/Scheduler.java (line 70) <https://reviews.apache.org/r/40638/#comment167269> Instead of simple String for jobID, Should we have a pojo similar to QueryHandle ? lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/Scheduler.java (line 71) <https://reviews.apache.org/r/40638/#comment167273> We should add getJobDetails - which returns SchedulerJobInfo. lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/Scheduler.java (line 134) <https://reviews.apache.org/r/40638/#comment167270> why is getAllJobStats taking jobId? Also lets add state, user, name, long fromDate and toDate as other filters for getAllJobStats. You can look at getAllQueries in QueryExecutionService lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/SchedulerJobInfo.java (line 31) <https://reviews.apache.org/r/40638/#comment167274> Shall we name the class ScheduleJobInfo instead of SchedulerJobInfo ? lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/SchedulerJobInfo.java (line 75) <https://reviews.apache.org/r/40638/#comment167276> Please add comments to all params in all classes lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/SchedulerJobInstanceInfo.java (line 29) <https://reviews.apache.org/r/40638/#comment167275> ScheduleJobInstanceInfo? - Amareshwari Sriramadasu On Nov. 24, 2015, 12:20 p.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40638/ > ----------------------------------------------------------- > > (Updated Nov. 24, 2015, 12:20 p.m.) > > > Review request for lens. > > > Bugs: LENS-879 > https://issues.apache.org/jira/browse/LENS-879 > > > Repository: lens > > > Description > ------- > > Base framework for lens scheduler service and job definition. > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/util/DateUtil.java PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/util/DateValidator.java > PRE-CREATION > lens-api/src/main/resources/example-job.xml PRE-CREATION > lens-api/src/main/resources/job-0.1.xsd PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/JobStats.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/QuerySchedulerService.java > 9f48d27 > > lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/Scheduler.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/SchedulerJobInfo.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/SchedulerJobInstanceInfo.java > PRE-CREATION > > lens-server/src/main/java/org/apache/lens/server/metrics/MetricsServiceImpl.java > 3389aba > > lens-server/src/main/java/org/apache/lens/server/scheduler/QuerySchedulerServiceImpl.java > 24949bd > > lens-server/src/main/java/org/apache/lens/server/scheduler/SchedulerService.java > PRE-CREATION > lens-server/src/main/resources/lensserver-default.xml 5f268cb > > lens-server/src/test/java/org/apache/lens/server/healthcheck/TestHealthChecks.java > e11413e > pom.xml b19857f > src/site/apt/admin/config.apt 88c1489 > > Diff: https://reviews.apache.org/r/40638/diff/ > > > Testing > ------- > > > Thanks, > > Ajay Yadava > >
