Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@aurora.incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============4046744838869108569=="
MIME-Version: 1.0
Subject: Re: Review Request 30891: Offer filtering for static vetoes. Part 3
 of 4: Offer filtering.
From: "Bill Farner" <wfarner@apache.org>
To: "Bill Farner" <wfarner@apache.org>, "Kevin Sweeney" <kevints@apache.org>
Cc: "Aurora" <reviews@aurora.incubator.apache.org>,
 "Maxim Khutornenko" <maxim@apache.org>
Date: Wed, 18 Feb 2015 01:32:54 -0000
Message-ID: <20150218013254.21353.97427@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Bill Farner" <noreply@reviews.apache.org>
References: <20150218005900.21353.50051@reviews.apache.org>
In-Reply-To: <20150218005900.21353.50051@reviews.apache.org>
Reply-To: "Bill Farner" <wfarner@apache.org>

--===============4046744838869108569==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> On Feb. 18, 2015, 12:59 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 316
> > <https://reviews.apache.org/r/30891/diff/3/?file=862798#file862798line316>
> >
> >     It seems like an abstraction violation for a thing named Queue to make logic decisions about its contents.  Have you considered having the `acceptor` own this behavior?
> 
> Maxim Khutornenko wrote:
>     Well, I don't really see why it has "Queue" in its name in the first place :) It does not really act like one. It accepts offers by iterating over the entire collection of available offers and sends Mesos launchTask requests. I'd rather rename it to OfferCache or OfferHandler if it helps.
>     
>     Regarding the abstraction violation, I actually see the static ban details belonging more to the OfferQueue than the acceptor as its more about offer state wrt a given task group than a specific assignment request.

> I actually see the static ban details belonging more to the OfferQueue than the acceptor as its more about offer state wrt a given task group than a specific assignment request

That makes sense, otherwise you need to figure out a way to plumb invalidation to the third party.

+1 to the rename


- Bill


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


On Feb. 13, 2015, 2:27 a.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30891/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2015, 2:27 a.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/OfferQueue.java 332338b7bef7d622333f8ea6508c4f5970b8e7c4 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java ced3bdef1731354aedc82bb12f45ba6f040e1ab7 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 
>   src/test/java/org/apache/aurora/scheduler/async/OfferQueueImplTest.java 2b5dc4902f57e508d76f5e16997ae09d04464220 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java d0e11932e8b5ba1393279137c8465a308e1d6bf5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 74e31334bc139c47eb8b0beee46ee7bad62a2f80 
> 
> Diff: https://reviews.apache.org/r/30891/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>


--===============4046744838869108569==--