----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/#review33522 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/base/Jobs.java <https://reviews.apache.org/r/17303/#comment63012> missing license header. please set this up in intellij so it's automatic. src/main/java/org/apache/aurora/scheduler/base/Jobs.java <https://reviews.apache.org/r/17303/#comment63017> Please doc. src/main/java/org/apache/aurora/scheduler/base/Tasks.java <https://reviews.apache.org/r/17303/#comment63018> ImmutableList.of(..) src/main/java/org/apache/aurora/scheduler/base/Tasks.java <https://reviews.apache.org/r/17303/#comment63019> Please fix premature wrap, and add @param, @return javadoc fields. Also, the term 'freshest' is ambiguous (and used interchangeably with 'latest' here and at a call site. How about getLatestTransitionedTask? Finally, please include a TODO in SchedulerThriftInterface to pull this method in there once the other caller is removed. src/main/thrift/org/apache/aurora/gen/api.thrift <https://reviews.apache.org/r/17303/#comment63015> Please give a more informative doc. As it stands, it provides the same info as "struct JobStats". src/main/thrift/org/apache/aurora/gen/api.thrift <https://reviews.apache.org/r/17303/#comment63014> As we discussed offline, please remove this. src/main/thrift/org/apache/aurora/gen/api.thrift <https://reviews.apache.org/r/17303/#comment63016> Just to make sure it's not dropped — this should be removed. src/test/java/org/apache/aurora/scheduler/base/JobsTest.java <https://reviews.apache.org/r/17303/#comment63013> missing license header src/test/java/org/apache/aurora/scheduler/base/TaskUtil.java <https://reviews.apache.org/r/17303/#comment63010> Do you think this class scales to multiple consumers? i.e. there's a bunch of hard-coded fields that work okay with the existing 2 callers, but we would need to plumb a lot more through for more broad usage. - Bill Farner On Jan. 24, 2014, 7:13 a.m., Suman Karumuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17303/ > ----------------------------------------------------------- > > (Updated Jan. 24, 2014, 7:13 a.m.) > > > Review request for Aurora, Kevin Sweeney and Bill Farner. > > > Bugs: AURORA-64 > https://issues.apache.org/jira/browse/AURORA-64 > > > Repository: aurora > > > Description > ------- > > Added task stats to getJobs API so it can be used by the role page in the UI. > Refactored code from SchedulerzRole and SchedulerzRoleTest into relevant > classes so it can be used by the UI and the thrift API. > Added tests for new code. > Moved populateJobConfig call into ReadOnlyScheduler. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/base/Jobs.java PRE-CREATION > src/main/java/org/apache/aurora/scheduler/base/Tasks.java > 06a19d80483b6949c9851b5d38fe34ac712aa75e > src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java > b0caca73b46fba928fb718ab45a608dad4685a2f > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > cf9099f307efa23ca34634e3512d9cdbebfa82f2 > src/main/thrift/org/apache/aurora/gen/api.thrift > 74010379baa2e47cefc228943f766c7b3a8b0d97 > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/base/TaskUtil.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/http/SchedulerzRoleTest.java > 912be189583419e7201e45650d18cd24a6a5a35b > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 6cefdfad469a9b69a5291ad46be1df14b443472e > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > 42fdca2759f15d007bee058485c237268c57597a > > Diff: https://reviews.apache.org/r/17303/diff/ > > > Testing > ------- > > gradle clean build > gradle run to test local UI. > > > Thanks, > > Suman Karumuri > >
