> On Feb. 3, 2014, 10:40 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/base/TaskUtil.java, line 19 > > <https://reviews.apache.org/r/17303/diff/1/?file=447781#file447781line19> > > > > 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. > > Suman Karumuri wrote: > I think so, since a lot of tests have a makeTask method in them and we > can get rid of some redundant code that way.
Emphasis of this question was about whether the class scales. Right now it works great because the two classes don't care about the fields you have hardcoded. However, this can quickly devolve into a bunch of factory methods, or methods with a ton of parameters. For now, i actually suggest accepting the duplication until we have an approach that prevents that situation. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17303/#review33522 ----------------------------------------------------------- On Feb. 26, 2014, 4:18 a.m., Suman Karumuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17303/ > ----------------------------------------------------------- > > (Updated Feb. 26, 2014, 4:18 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 getJobSummary API so it can be used by the role and role/environment > 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 > d9cb886ef333e108d5d5f86043ac80e450689894 > src/main/java/org/apache/aurora/scheduler/http/SchedulerzRole.java > 25ba7da5f8bbe5416f41bb0b14850beb84392cc7 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > dccae15a2c529025c9bb6a6f7a0220779ca4f9a1 > src/main/thrift/org/apache/aurora/gen/api.thrift > cd60f47bf34b4a634004e2ad9eadad37aa1556bb > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java PRE-CREATION > src/test/java/org/apache/aurora/scheduler/http/SchedulerzRoleTest.java > 912be189583419e7201e45650d18cd24a6a5a35b > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 4a2d39d8b25c4a6b161c47d6ba7068d74f8a60e0 > src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java > 1edc0d7b224cc477ea6e8873e76ee8c70c6b4d50 > src/test/resources/org/apache/aurora/gen/api.thrift.md5 > fafb5100443482e662db453429c5259f2ab80ae5 > > Diff: https://reviews.apache.org/r/17303/diff/ > > > Testing > ------- > > gradle clean build > gradle run to test local UI. > > > Thanks, > > Suman Karumuri > >
