> On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInfo.java, > > line 38 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148018#file1148018line38> > > > > Is this not ScheduleJobHandle ?
Yes. It changed after review request and I forgot to update. Will fix it. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInfo.java, > > lines 47-87 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148018#file1148018line47> > > > > I think all of these fields should be replaced with single object XJob Agreed, will fix. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceInfo.java, > > line 33 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148020#file1148020line33> > > > > Should be SchedulerJobInstanceHandle Yes. Will fix it. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceInfo.java, > > line 36 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148020#file1148020line36> > > > > Why is this new session handle. Should be "session handle in which > > instance was run" The thought was that the setter gets the new value which needs to be set. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceInfo.java, > > line 39 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148020#file1148020line39> > > > > Return LensSessionHandle ? Will fix. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceInfo.java, > > line 42 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148020#file1148020line42> > > > > StartTime - when the instance started to run. > > > > I see all other comments are also saying the values to be set. Instead > > they are actually when it ran. These are the documentations which will be generated on the setter methods. You pass in the value which you want to be set and hence the language. @returns documentation is exactly what you have suggested. I think it is correct, let me know if you think otherwise. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/resources/scheduler-job-0.1.xsd, line 47 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148021#file1148021line47> > > > > Should be "Name of the scheduled job" ? Yes. Will update. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/resources/scheduler-job-0.1.xsd, line 102 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148021#file1148021line102> > > > > If we are separating session into separate type, resource_path should > > be actually part of session. Sure. Fixed it. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/resources/scheduler-job-0.1.xsd, line 117 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148021#file1148021line117> > > > > Can we be consistent with names for types and elements? All types can > > start with x_type and elements would contain actual names and no x_ ? Definitely. Sorry for the oversight. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/resources/scheduler-job-0.1.xsd, line 153 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148021#file1148021line153> > > > > Comment needs to be updated. Will fix. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/resources/scheduler-job-0.1.xsd, line 122 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148021#file1148021line122> > > > > Can you change the comment to "Database name to be used in session" ? Sure. Will update. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceInfo.java, > > line 81 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148020#file1148020line81> > > > > I dont think we would allow modification of an instance, so should not > > be required. Yes. will fix > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInfo.java, > > line 55 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148018#file1148018line55> > > > > we should return XJob instead of Clob. Makes sense, will fix. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInfo.java, > > line 63 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148018#file1148018line63> > > > > Instead of Clob we should return correspondig objects. Applies for all > > Clob objects. Yes, will fix. > On Nov. 30, 2015, 5:16 a.m., Amareshwari Sriramadasu wrote: > > lens-examples/src/main/resources/example-job.xml, line 1 > > <https://reviews.apache.org/r/40638/diff/3/?file=1148022#file1148022line1> > > > > Can we update the client packaging to include schedule-examples/ ? The files under resources don't follow a fixed convention for prefix or suffix and hence it is not possible to separate the scheduler examples from others using a few generic regular expressions e.g. there are files like city.xml, customer.xml etc. If you want then I can try solving it in a separate JIRA. - Ajay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40638/#review108280 ----------------------------------------------------------- On Nov. 27, 2015, 7:46 p.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40638/ > ----------------------------------------------------------- > > (Updated Nov. 27, 2015, 7:46 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/query/SchedulerJobHandle.java > PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInfo.java > PRE-CREATION > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceHandle.java > PRE-CREATION > > lens-api/src/main/java/org/apache/lens/api/query/SchedulerJobInstanceInfo.java > PRE-CREATION > lens-api/src/main/resources/scheduler-job-0.1.xsd PRE-CREATION > lens-examples/src/main/resources/example-job.xml 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/SchedulerJobStats.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/scheduler/SchedulerService.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/SchedulerServiceImpl.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 > >
