aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
Date Mon, 23 Feb 2015 20:35:49 GMT


> On Feb. 21, 2015, 12:43 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java, line
80
> > <https://reviews.apache.org/r/30891/diff/5-6/?file=867194#file867194line80>
> >
> >     This is kinda weird, what's the motivation here?
> 
> Maxim Khutornenko wrote:
>     Test coverage for the fine-logging statements.
> 
> Maxim Khutornenko wrote:
>     Just noticed you are referring to the `setUp()`. This is making sure anything with
INFO is still logged and setting to FINE does not mess up with it depending on the test execution
sequence.
> 
> Bill Farner wrote:
>     How about an injected `Logger` instead?  Guice actually injects this by default,
and there's no good reason we aren't using it.
>     
>     https://github.com/google/guice/wiki/BuiltInBindings

It would require modifying HostOffers to accept a Logger instance and adjusting both OfferManagerImplTest
and TaskSchedulerTest to expect info(), fine() and isLoggable(). I rejected that idea to reduce
the diff size. 

Since info level folds into fine, set the default level to Fine in setUp() instead.


- Maxim


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


On Feb. 20, 2015, 11:58 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30891/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2015, 11:58 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Bill Farner.
> 
> 
> Bugs: AURORA-909
>     https://issues.apache.org/jira/browse/AURORA-909
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers.
> 
> Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent.
> 
> Original RB: https://reviews.apache.org/r/28617/
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/OfferManager.java b241d7b22c3d1ceca127b2671eb608ae41283bf3

>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 21ea7d2b9d2f8c76367d7ae985270402bb51ea26

>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917

>   src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff

>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 9eef52a333e09454e8fd0026371c7e64472a883d

>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java b6d4d8e771c7d16a46e43c7d5c427b911f8b661d

> 
> Diff: https://reviews.apache.org/r/30891/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


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