Re: Review Request 25459: Adding quota check into startJobUpdate.

2014-09-12 Thread Maxim Khutornenko
> On Sept. 12, 2014, 11:04 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java, > > line 102 > > > > > > You mean inclusive? Not really. Neither 0 nor max value makes

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53244 --- Ship it! src/main/resources/org/apache/aurora/scheduler/http/ui/js

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Maxim Khutornenko
> On Sept. 11, 2014, 11:41 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 398 > > > > > > Spent quite a bit of time reading this method. Wou

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/#review53243 --- Ship it! Ship It! - Bill Farner On Sept. 12, 2014, 9:43 p.m., Ke

Re: Review Request 25543: Update to pants 0.0.23.

2014-09-12 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25543/#review53242 --- Ship it! awesome- thanks! - Joe Smith On Sept. 11, 2014, 9:13 a.

Re: Review Request 25459: Adding quota check into startJobUpdate.

2014-09-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25459/#review53238 --- src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.

Re: Review Request 25543: Update to pants 0.0.23.

2014-09-12 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25543/#review53239 --- Ship it! I noticed you just commented out some of the timeout= keyw

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/ --- (Updated Sept. 12, 2014, 10:54 p.m.) Review request for Aurora, Joshua Cohen, K

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-12 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 12, 2014, 10:39 p.m.) Review request for Aurora, Joshua Cohen, K

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
> On Sept. 11, 2014, 11:41 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/base/Query.java, line 104 > > > > > > typo Better yet - addressed TODO :-) > On Sept. 11, 2014, 11:41 p.m., Max

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-12 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/ --- (Updated Sept. 12, 2014, 10:32 p.m.) Review request for Aurora, Joshua Cohen, K

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-12 Thread David McLaughlin
> On Sept. 12, 2014, 9:20 p.m., Joshua Cohen wrote: > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/filters.js, lines > > 69-72 > > > > > > This might read a little bit cleaner if you chained it all? > >

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/#review53226 --- Ship it! Ship It! - Joshua Cohen On Sept. 12, 2014, 9:43 p.m., K

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/ --- (Updated Sept. 12, 2014, 2:43 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney
> On Sept. 12, 2014, 2:37 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java, > > lines 369-373 > > > > > > Obviously not related, but should these all use Objects.requ

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/ --- (Updated Sept. 12, 2014, 2:39 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
> On Sept. 11, 2014, 10:13 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java, > > line 111 > > > > > > Instead of embedding the logic into a switch here, would

Re: Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/#review53220 --- Ship it! lgtm src/main/java/org/apache/aurora/scheduler/http/Jett

Review Request 25593: Use a ServletContextListener to configure servlets

2014-09-12 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25593/ --- Review request for Aurora, David McLaughlin and Bill Farner. Repository: aurora

Re: Review Request 25259: Add update information to the scheduler UI

2014-09-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53216 --- Mostly just nitpicky style/readability stuff... src/main/resources

Re: Review Request 25455: Deprecating PopulateJobResult.populated thrift field.

2014-09-12 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25455/ --- (Updated Sept. 12, 2014, 6:35 p.m.) Review request for Aurora, Bill Farner and

Re: Review Request 25529: Add a controller for job updates.

2014-09-12 Thread Bill Farner
> On Sept. 11, 2014, 6:51 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java, > > line 109 > > > > > > Would it be a bad idea to encapsulate this into a Instance

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread Joe Smith
> On Sept. 12, 2014, 10:09 a.m., Joe Smith wrote: > > src/test/python/apache/aurora/client/cli/test_task_run.py, line 228 > > > > > > "Test the ssh command for proper behavior when no tasks are found > > within a job"

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/#review53189 --- Ship it! Ship It! - Joe Smith On Sept. 12, 2014, 10:17 a.m., Mar

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread David McLaughlin
> On Sept. 12, 2014, 5:09 p.m., Joe Smith wrote: > > src/test/python/apache/aurora/client/cli/test_task_run.py, line 228 > > > > > > "Test the ssh command for proper behavior when no tasks are found > > within a job"

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

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

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread Joshua Cohen
> On Sept. 12, 2014, 5:09 p.m., Joe Smith wrote: > > src/test/python/apache/aurora/client/cli/test_task_run.py, line 228 > > > > > > "Test the ssh command for proper behavior when no tasks are found > > within a job"

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/#review53184 --- src/test/python/apache/aurora/client/cli/test_task_run.py

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/#review53182 --- Ship it! src/main/python/apache/aurora/client/cli/task.py

Re: Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/#review53181 --- Ship it! Ship It! - Zameer Manji On Sept. 12, 2014, 7:34 a.m., M

Review Request 25582: Fix error in client "task ssh" command when the job isn't found.

2014-09-12 Thread Mark Chu-Carroll
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25582/ --- Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-706