aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 17303: Added getJobSummary API
Date Fri, 28 Feb 2014 20:00:51 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message