> On Nov. 11, 2014, 10:04 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 360
> > <https://reviews.apache.org/r/27705/diff/1/?file=753241#file753241line360>
> >
> >     It's nice when boolean-returning methods have names that self-document 
> > what the return value is.  In this case, something like `accceptsOffer`.

Renamed.


> On Nov. 11, 2014, 10:04 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 362
> > <https://reviews.apache.org/r/27705/diff/1/?file=753241#file753241line362>
> >
> >     odd line break here, don't split up the type parameter.

no idea how that happened, fixed.


> On Nov. 11, 2014, 10:04 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java, 
> > line 81
> > <https://reviews.apache.org/r/27705/diff/1/?file=753244#file753244line81>
> >
> >     s/rawReason/reasonParameter/?
> >     
> >     Additionally, this should probably be `String...` to avoid a future yak 
> > shave to support non-unary argument vetoes.

Renamed. 

I don't see a use case for String... just yet. Given this is a private 
constructor it should be easy to add when needed.


> On Nov. 11, 2014, 10:04 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java, 
> > line 283
> > <https://reviews.apache.org/r/27705/diff/1/?file=753245#file753245line283>
> >
> >     Can you do this in a SchedulingFilter decorator instead?  That way you 
> > can hide a bunch of logic from SchedulingFilterImpl.
> 
> Bill Farner wrote:
>     Oh, looks like the hard work may already be done - can you consume the 
> `Vetoed` pubsub event elsewhere?

Sure, moved to TaskVars.


- Maxim


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27705/#review60861
-----------------------------------------------------------


On Nov. 6, 2014, 10:23 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27705/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2014, 10:23 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-914
>     https://issues.apache.org/jira/browse/AURORA-914
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Adding @Timed to trace scheduling latencies and Veto counters per type.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java 
> 1a45d08c8854a335161476c8321c751f763dc12e 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> b23457e0e64b490297166131a1b1b51b6d330415 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
> 3839083f27ca5d4b93406152559b58b04e912a10 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> c37272c9f46c086cb57b79a5202b3bd80e156f07 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 0533baa5e90ca62b8d35ba05474eaa8e27741a5a 
>   src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java 
> 87203690f09456ac1ca5e9da2b82826d60cbd723 
>   src/main/java/org/apache/aurora/scheduler/stats/CachedCounters.java 
> aaedb3b5ec2cb27550449435efa8f335c6a9baad 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> c0fa462c0ebe0b06fa354f5f63d5965827c669a1 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  0318179cd70661890f5a53908d1985d54474d476 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  bffbf83653535ecd9bf7b149e1e564c5fba56d17 
>   src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 
> b60b004adbd6753ec6fef125fd70286be5071c56 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  b42f6e2d23eb44605cc84ce01a80f10344a7e43f 
> 
> Diff: https://reviews.apache.org/r/27705/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> Verified new stats in vagrant.
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to