Re: Review Request 25255: Implement server-driven update commands.

2014-09-17 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/ --- (Updated Sept. 17, 2014, 1:06 p.m.) Review request for Aurora. Repository: au

Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository:

Re: Review Request 25255: Implement server-driven update commands.

2014-09-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/#review53693 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 17, 2014, 5:06 p.

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53694 --- Does removing an instance take any significant amount of time? Can i

Re: Review Request 25721: Asynchronous JS for Scheduler UI

2014-09-17 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/ --- (Updated Sept. 17, 2014, 5:49 p.m.) Review request for Aurora, Joshua Cohen, Ke

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Bill Farner
> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote: > > Does removing an instance take any significant amount of time? Can it fail? > > The concern is we'll be showing 100% progress but the update is still > > running when reducing the instance count. Depends on your definition of signific

Re: Review Request 25721: Asynchronous JS for Scheduler UI

2014-09-17 Thread David McLaughlin
> On Sept. 17, 2014, 4:34 a.m., Bill Farner wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, > > line 368 > > > > > > More objective? I don't want to kill the humor, but an objecti

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread David McLaughlin
> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote: > > Does removing an instance take any significant amount of time? Can it fail? > > The concern is we'll be showing 100% progress but the update is still > > running when reducing the instance count. > > Bill Farner wrote: > Depends o

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Bill Farner
> On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote: > > Does removing an instance take any significant amount of time? Can it fail? > > The concern is we'll be showing 100% progress but the update is still > > running when reducing the instance count. > > Bill Farner wrote: > Depends o

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53706 --- Ship it! Ship It! - David McLaughlin On Sept. 17, 2014, 5:15 p.m

Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states.

2014-09-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53708 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 17, 2014, 5:15 p.

Re: Review Request 25255: Implement server-driven update commands.

2014-09-17 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/ --- (Updated Sept. 17, 2014, 2:44 p.m.) Review request for Aurora. Changes --

Re: Review Request 25721: Asynchronous JS for Scheduler UI

2014-09-17 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25721/#review53733 --- Ship it! Ship It! - Joshua Cohen On Sept. 17, 2014, 5:49 p.m., D

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25750/ --- (Updated Sept. 17, 2014, 8:40 p.m.) Review request for Aurora, David McLaughlin

Review Request 25753: Surface instance update status changes so they may be persisted.

2014-09-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25753/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository:

Re: Review Request 25753: Surface instance update status changes so they may be persisted.

2014-09-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25753/ --- (Updated Sept. 17, 2014, 8:55 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 25753: Surface instance update status changes so they may be persisted.

2014-09-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25753/#review53743 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/OneWayJ

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25750/#review53746 --- +1 to the code change, UI stuff in particular looks good to me. -1

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread Bill Farner
> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote: > > +1 to the code change, UI stuff in particular looks good to me. > > > > -1 to dropping instanceCount completely though. I thought we mentioned we > > wanted to capture and store all the original details the user sends? Even > > if thi

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread Maxim Khutornenko
> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote: > > +1 to the code change, UI stuff in particular looks good to me. > > > > -1 to dropping instanceCount completely though. I thought we mentioned we > > wanted to capture and store all the original details the user sends? Even > > if thi

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread Maxim Khutornenko
> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote: > > +1 to the code change, UI stuff in particular looks good to me. > > > > -1 to dropping instanceCount completely though. I thought we mentioned we > > wanted to capture and store all the original details the user sends? Even > > if thi

Re: Review Request 25753: Surface instance update status changes so they may be persisted.

2014-09-17 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25753/#review53753 --- Ship it! Other than the docs, lgtm. - David McLaughlin On Sept.

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread David McLaughlin
> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote: > > +1 to the code change, UI stuff in particular looks good to me. > > > > -1 to dropping instanceCount completely though. I thought we mentioned we > > wanted to capture and store all the original details the user sends? Even > > if thi

Re: Review Request 25750: Store new task configuration in JobUpdateConfiguration as InstanceTaskConfig

2014-09-17 Thread Maxim Khutornenko
> On Sept. 17, 2014, 9:18 p.m., David McLaughlin wrote: > > +1 to the code change, UI stuff in particular looks good to me. > > > > -1 to dropping instanceCount completely though. I thought we mentioned we > > wanted to capture and store all the original details the user sends? Even > > if thi

Review Request 25760: Performance improvements and instrumentation for snapshot

2014-09-17 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25760/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-722

Re: Review Request 25760: Performance improvements and instrumentation for snapshot

2014-09-17 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25760/#review53778 --- Ship it! src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.j

Re: Review Request 25760: Performance improvements and instrumentation for snapshot

2014-09-17 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25760/#review53786 --- Ship it! LGTM mod Maxim's comments. - Bill Farner On Sept. 18, 2