Re: Review Request 25543: Update to pants 0.0.23.

2014-09-15 Thread Mark Chu-Carroll
On Sept. 12, 2014, 6:54 p.m., Brian Wickman wrote: I noticed you just commented out some of the timeout= keywords -- do you plan to remove those or just leave them as annotations? Leave them as annotations. I'm not really clear on why the timeout was removed, but the fact that we believe

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

2014-09-15 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25255/#review53347 --- src/main/python/apache/aurora/client/cli/standalone_client.py

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

2014-09-15 Thread Mark Chu-Carroll
On Sept. 15, 2014, 12:04 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/standalone_client.py, line 121 https://reviews.apache.org/r/25255/diff/2/?file=685554#file685554line121 I am still not sure why we are going with the top level noun for job updates.

Re: Review Request 25175: Fix possible deadlock in TaskRunner.collect_updates.

2014-09-15 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/ --- (Updated Sept. 15, 2014, 5:52 p.m.) Review request for Aurora. Changes

Re: Review Request 25175: Fix possible deadlock in TaskRunner.collect_updates.

2014-09-15 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/ --- (Updated Sept. 15, 2014, 6:07 p.m.) Review request for Aurora. Bugs:

Re: Review Request 25175: Fix possible deadlock in TaskRunner.collect_updates.

2014-09-15 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/ --- (Updated Sept. 15, 2014, 6:08 p.m.) Review request for Aurora and Kevin

Re: Review Request 25175: Fix possible deadlock in TaskRunner.collect_updates.

2014-09-15 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/#review53367 --- Ship it! Ship It! - Joe Smith On Sept. 15, 2014, 11:08 a.m.,

Re: Review Request 25653: Added store APIs to query for IJobUpdate and IJobUpdateConfiguration.

2014-09-15 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25653/#review53379 --- Ship it!

Re: Review Request 25175: Fix possible deadlock in TaskRunner.collect_updates.

2014-09-15 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25175/#review53385 --- Ship it! Please circle back and add test coverage - Kevin Sweeney

Re: Review Request 25653: Added store APIs to query for IJobUpdate and IJobUpdateConfiguration.

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

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

2014-09-15 Thread Mark Chu-Carroll
The choices are: (1) Action parameter: aurora job update --action=start/pause/resume/abort. Pros: it's part of the job noun and the familiar update verb. Cons: - the action parameter is ugly, - we lose automatic parameter checking, since the different actions take different parameters. - it's

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

2014-09-15 Thread Maxim Khutornenko
I guess there is also the option you originally proposed: aurora job update --start|--pause|--resume|--abort. If you don't like the option syntax, would it be too much work to support double verb (or rather double noun) syntax, like aurora job update start? On Mon, Sep 15, 2014 at 2:07 PM, Mark

Re: Review Request 25667: Removing host_drain delay.

2014-09-15 Thread Joe Smith
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25667/#review53419 --- Ship it! Ship It! - Joe Smith On Sept. 15, 2014, 3:04 p.m.,

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

2014-09-15 Thread Mark Chu-Carroll
As I said - the version with the sub-actions fouls up parameter checking. (That's the action parameter option. It doesn't matter whether it's --action=start or --start; in fact, the --action at least lets the parameter parser do a little bit of checking.) It's possible to add another layer of

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

2014-09-15 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25259/#review53422 --- Ship it! Thanks for adding those subset-related comments, clearer

Review Request 25673: Fixing broken test.

2014-09-15 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25673/ --- Review request for Aurora, Joe Smith and Bill Farner. Repository: aurora

Re: Review Request 25673: Fixing broken test.

2014-09-15 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25673/#review53428 --- Ship it! Ship It! - Bill Farner On Sept. 15, 2014, 10:56 p.m.,

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

2014-09-15 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/#review53425 --- Ship it! Looks good to me, just a few minor questions/nits:

Review Request 25671: Instruct thrift to generate private fields in java.

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

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

2014-09-15 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25529/#review53389 ---

Re: Review Request 25671: Instruct thrift to generate private fields in java.

2014-09-15 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25671/#review53432 --- Ship it! Ship It! - Kevin Sweeney On Sept. 15, 2014, 4:02 p.m.,

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

2014-09-15 Thread Bill Farner
On Sept. 15, 2014, 11:01 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1368 https://reviews.apache.org/r/25529/diff/3/?file=689752#file689752line1368 Is this a leftover from some intermediate work? Not sure how

Re: Review Request 25671: Instruct thrift to generate private fields in java.

2014-09-15 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25671/#review53435 --- Ship it! Ship It! - Zameer Manji On Sept. 15, 2014, 4:02 p.m.,

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

2014-09-15 Thread Bill Farner
On Sept. 15, 2014, 11:13 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, line 1368 https://reviews.apache.org/r/25529/diff/3/?file=689752#file689752line1368 If I'm reading correctly there's no need to make this final

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

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

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

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

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

2014-09-15 Thread Bill Farner
On Sept. 15, 2014, 11:13 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java, line 61 https://reviews.apache.org/r/25529/diff/3/?file=689761#file689761line61 TODO to use AssistedInject to generate this factory here?

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

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