> On Jan. 19, 2018, 10:12 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java
> > Line 66 (original), 65 (patched)
> > <https://reviews.apache.org/r/65233/diff/1/?file=1942312#file1942312line66>
> >
> >     Inject `OfferSet` rather than `OfferSettings`.
> 
> Jordan Ly wrote:
>     I still need to inject OfferSettings for making the static ban cache -- 
> is it fine if we keep piggybacking OfferSet on OfferSettings for now or do 
> you mean I should inject both separately.

Ah, i missed the other input.  I'm fine to keep it as-is.


> On Jan. 19, 2018, 10:12 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java
> > Lines 23 (patched)
> > <https://reviews.apache.org/r/65233/diff/1/?file=1942317#file1942317line23>
> >
> >     Up to you, but i'm not seeing a good reason to return `Iterable` over 
> > `Stream` here and below.  It has the upside of no extra hoops for filtering 
> > and no need to protect from modification.
> 
> Jordan Ly wrote:
>     I agree that returning a Stream would be nicer, but I would like to keep 
> the current interfaces below stable for this patch.
>     
>     I would like to do a larger refactor later (similar to your switch from 
> Gauava to Java optionals) to target iterables and streams.

sgtm


- Bill


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


On Jan. 19, 2018, 10:57 a.m., Jordan Ly wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65233/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 10:57 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Stephan Erb, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The goal of this patch is to provide a more reasonable abstraction for custom 
> scheduling.
> 
> Currently, we have `OfferSelector`, `OfferOrder`, and recently I proposed 
> `FilterableOfferCollection` (https://reviews.apache.org/r/65225/). These were 
> all created in order to provide more customization within the scheduling 
> loop. However, they suffer from being leaky and too disparate. This patch 
> hopes to combine all of those components into a single `OfferSet` which can 
> be injected and used by HostOffers. This interface allows for custom 
> scheduling logic to be co-located with custom data structures for `offers` as 
> opposed to being passed around as different components.
> 
> The following options will be removed from the last 0.19 to now:
> ```
> -offer_order
> -offer_order_modules
> ```
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 3053e543793669601ec9132adae376bcdce64c32 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> f44cb61f5d1805aeb0f98135453dbc82f3bc9ad4 
>   src/main/java/org/apache/aurora/scheduler/config/CliOptions.java 
> e4e535824adb86c98c6173a20f97a9a90b08483a 
>   src/main/java/org/apache/aurora/scheduler/offers/HostOffers.java 
> 2ea7a01085b87c8ed6765537a8005e2349784ab0 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 8f9e33d81be9087821784a8a08079a1736d9cb63 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManagerModule.java 
> de16c14c887d4225e0629b1580eb5e740798f1f7 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferOrder.java 
> 7c99c309e0e97519719905f725bb9f6a59d2a7f5 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferOrderBuilder.java 
> 1260ef19506acb8e8a937d4fd7b7152361bd3c40 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSet.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSetImpl.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java 
> 1e36b2c3094e99f05aa4a4f098a48df2293b4320 
>   
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelector.java
>  5f4f452cb133e2be73f881700a54ba4c8e60a35a 
>   
> src/main/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorModule.java
>  4d36487f430483444314a8cefd133e7c766075de 
>   src/main/java/org/apache/aurora/scheduler/scheduling/OfferSelector.java 
> 6f5d55c5a586ac923b5f375d1935ec2049673ec0 
>   src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImpl.java 
> ec416cc12b2fb4a11a273c77dbe72b79f96fa7e1 
>   
> src/main/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplModule.java
>  d101a497896ba3b8ca7984a63f861494cbddbb35 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 
> c84fad3b615c49ca7527ae3f7fa116aa8f90ff0c 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 28224a5c587b235ccc86a286e796eeca66929232 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/FirstFitOfferSelectorTest.java
>  e6b2b7465fb4fd432cda50d1e714dfc587764689 
>   
> src/test/java/org/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java
>  f84941edd96d974a6c2d7e087324bc8fbbac3b0a 
> 
> 
> Diff: https://reviews.apache.org/r/65233/diff/3/
> 
> 
> Testing
> -------
> 
> `./gradlew test`
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> Testing injection of a custom `OfferSet` onto a test cluster.
> 
> 
> Thanks,
> 
> Jordan Ly
> 
>

Reply via email to