Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/#review71999 --- src/main/resources/scheduler/assets/js/controllers.js

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-11 Thread David McLaughlin
> On Feb. 11, 2015, 6:58 p.m., David McLaughlin wrote: > > src/main/resources/scheduler/assets/js/controllers.js, line 338 > > > > > > You should never have to do this with routes in Angular. I think this > > is a cod

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > lines 282-292 > > > > > > There's a race on access to `pulseStates`. Terminati

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72016 --- Ship it! Ship It! - Bill Farner On Feb. 11, 2015, 7:19 p.m., Max

Re: Review Request 28617: Implemented offer filtering for tasks with static vetoes.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 5, 2015, 12:01 a.m., Bill Farner wrote: > > Can you see any opportunity to break this diff apart? As it stands i'm > > having a hard time giving a thoughtful review. Perhaps you can start by > > introducing the `Assignment` class? > > Maxim Khutornenko wrote: > I'd really prefe

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72014 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/JobUpda

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72022 --- Ship it! Master (7b531e9) is green with this patch. ./build-suppo

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72026 --- Ship it! src/main/java/org/apache/aurora/scheduler/state/TaskAssig

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 8:46 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 276 > > > > > > can drop the else? Sure. - Maxim ---

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 10:01 p.m.) Review request for Aurora, David McLaughlin

Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72028 --- Ship it! Ship It! - Joshua Cohen On Feb. 11, 2015, 10:01 p.m., M

Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora

Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909

Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72030 --- src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72036 --- This patch does not apply cleanly on master (7b531e9), do you need t

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 9:59 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 64 > > > > > > fits on one line > > > > ditto elsewhere in this file Fixed. > On

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- (Updated Feb. 11, 2015, 10:52 p.m.) Review request for Aurora, Kevin Sweeney an

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72041 --- Ship it! Master (7b531e9) is green with this patch. ./build-suppo

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 10:22 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 316 > > > > > > The cohesion of the HostOffer class seems to be rather low. The new > > fiel

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72043 --- Master (7b531e9) is red with this patch. ./build-support/jenkins/b

Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/#review72048 --- Ship it! Master (7b531e9) is green with this patch. ./build-suppo

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 10, 2015, 7:28 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > lines 1609-1618 > > > > > > I don't know how you feel about the need for a Su

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 10, 2015, 11:57 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > lines 1609-1618 > > > > > > s/final// Done. - Maxim

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 2:13 a.m., Bill Farner wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 630 > > > > > > s/Key// Done and done. - Maxim -

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:35 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/#review72058 --- Ship it! Master (7b531e9) is green with this patch. ./build-suppo

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72059 --- This patch does not apply cleanly on master (7b531e9), do you need t

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72060 --- Ship it! LGTM. I wouldn't consider the below issue a blocker. src

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 2:23 a.m., Aurora ReviewBot wrote: > > Master (64fa0ca) is red with this patch. > > ./build-support/jenkins/build.sh > > > >  self.expect_finish() > >  self.replay_mocks() > > 

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 11:44 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > lines 1596-1603 > > > > > > It seems strange that the UPDATE_COORDINATOR

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:51 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

2015-02-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/#review72061 --- src/main/java/org/apache/aurora/scheduler/TaskVars.java

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72065 --- Ship it! pending clean test run from review bot. - Joshua Cohen

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72068 --- Ship it! FWIW I do agree with Bill's comment that minimum pulse tim

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Maxim Khutornenko
> On Feb. 11, 2015, 11:59 p.m., David McLaughlin wrote: > > FWIW I do agree with Bill's comment that minimum pulse time should be > > enforced in scheduler rather than here. I am +1 on the scheduler check as complementary to the client when/if there is a need for it. I am -1 on using scheduler

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72071 --- Ship it! Master (b8f71fb) is green with this patch. ./build-suppo

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72091 --- @ReviewBot retry - Maxim Khutornenko On Feb. 11, 2015, 1:19 a.m.,

Review Request 30913: Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1119

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72098 --- Master (61e6c35) is red with this patch. ./build-support/jenkins/b

Review Request 30915: Increase findbugs heap size.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/ --- Review request for Aurora and Bill Farner. Repository: aurora Description ---

Re: Review Request 30915: Increase findbugs heap size.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/#review72103 --- Ship it! Master (61e6c35) is green with this patch. ./build-suppo

Re: Review Request 30913: Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/#review72104 --- Ship it! Master (61e6c35) is green with this patch. ./build-suppo

Re: Review Request 30710: add mesos role feature

2015-02-11 Thread lozh...@ebay.com zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/ --- (Updated Feb. 12, 2015, 6:12 a.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 30710: add mesos role feature

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/#review72113 --- Ship it! Master (61e6c35) is green with this patch. ./build-suppo