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:

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

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

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,

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 significant.

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 https://reviews.apache.org/r/25721/diff/1/?file=691307#file691307line368 More objective? I don't want to kill the humor, but an objective

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 on your

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 on your

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

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

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.,

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

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

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 this is

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 this is

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 this is

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 this is

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 this is

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!

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,