Re: Review Request 26004: Add aurora update list and aurora update status commands.

2014-09-29 Thread Mark Chu-Carroll
On Sept. 24, 2014, 6:28 p.m., David McLaughlin wrote: What was the rationale for hiding update IDs from the user and making job key the parameter for update status? It's nice you can quickly see if a job key has an update in progress.. but what happens if you're wanting to see a

Review Request 26137: Fix help for new update command.

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

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

2014-09-29 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54825 --- I've got an updated diff for this, but reviewboard is choking when

Re: Review Request 26095: Fix instance summary visualisation.

2014-09-29 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26095/#review54829 --- Ship it! Ship It! - Maxim Khutornenko On Sept. 29, 2014, 4:11

Re: Review Request 26095: Fix instance summary visualisation.

2014-09-29 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26095/#review54830 --- Ship it! Ship It! - Bill Farner On Sept. 29, 2014, 4:11 p.m.,

Re: Review Request 26095: Fix instance summary visualisation.

2014-09-29 Thread Joshua Cohen
On Sept. 26, 2014, 8:19 p.m., Joshua Cohen wrote: src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, line 440 https://reviews.apache.org/r/26095/diff/1/?file=706256#file706256line440 As far as I can see, allInstances is only used here to get the length, can

Re: Review Request 26095: Fix instance summary visualisation.

2014-09-29 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26095/#review54833 --- Ship it! Looks good to me. - Joshua Cohen On Sept. 29, 2014,

Re: Review Request 23330: replace 143 kB favicon with 318 byte version

2014-09-29 Thread David Robinson
On Sept. 29, 2014, 4:31 p.m., Brian Wickman wrote: david -- could you publish a remote branch somewhere that i can use to merge this to master? the reviewboard diff does not contain the .ico and ./rbt patch does not support binary diffs.

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

2014-09-29 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54854 --- Ship it! Ship It! - David McLaughlin On Sept. 19, 2014, 9:39

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

2014-09-29 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54859 --- Ship it! Ship It! - Bill Farner On Sept. 19, 2014, 9:39 p.m.,

Re: Review Request 26102: Dropping assert on empty desired instances set.

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

Re: Review Request 26123: Fail the build on lack of test coverage.

2014-09-29 Thread Bill Farner
On Sept. 28, 2014, 4:05 a.m., Joshua Cohen wrote: build.gradle, line 571 https://reviews.apache.org/r/26123/diff/1/?file=707768#file707768line571 This seems like a potentially harsh penalty for good behavior if someone has to go from 0 to $MIN_COVERAGE in one go... I

Re: Review Request 26123: Fail the build on lack of test coverage.

2014-09-29 Thread Bill Farner
On Sept. 27, 2014, 11:38 p.m., Zameer Manji wrote: Once this is commited, please make tickets for adding tests to these classes. I'm not sure how to best do this without either creating a ton of tickets that are bound to be forgotten, or a monster ticket that is difficult to track. Any

Re: Review Request 26123: Fail the build on lack of test coverage.

2014-09-29 Thread Zameer Manji
On Sept. 27, 2014, 4:38 p.m., Zameer Manji wrote: Once this is commited, please make tickets for adding tests to these classes. Bill Farner wrote: I'm not sure how to best do this without either creating a ton of tickets that are bound to be forgotten, or a monster ticket that is

Re: Review Request 26123: Fail the build on lack of test coverage.

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

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

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

Re: Review Request 26113: Upgrade to gradle 2.1

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26113/#review54889 --- Ship it! build.gradle

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review54890 --- Pinging Brian and Kevin to this review. - Zameer Manji On Sept.

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Sept. 29, 2014, 2:46 p.m.) Review request for Aurora, Kevin Sweeney,

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review54892 --- src/main/python/apache/aurora/executor/common/announcer.py

Re: Review Request 26137: Fix help for new update command.

2014-09-29 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review54894 --- Ship it! Ship It! - Zameer Manji On Sept. 29, 2014, 8:05 a.m.,

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review54895 --- src/main/python/apache/aurora/executor/common/announcer.py

Re: Review Request 24063: Use JCenter over HTTPS instead of Maven Central

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24063/#review54896 --- Discarding this since Gradle 2.1 uses HTTPS - Kevin Sweeney On

Re: Review Request 26154: Stop sending and receiving DeletedTasks framework message.

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26154/#review54898 --- Ship it!

Re: Review Request 26113: Upgrade to gradle 2.1

2014-09-29 Thread Bill Farner
On Sept. 29, 2014, 9:46 p.m., Kevin Sweeney wrote: build.gradle, line 58 https://reviews.apache.org/r/26113/diff/1/?file=707643#file707643line58 I think since this is groovy you can do it.name != 'compileGeneratedJava' instead? Fixed. - Bill

Re: Review Request 26113: Upgrade to gradle 2.1

2014-09-29 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26113/ --- (Updated Sept. 29, 2014, 10:33 p.m.) Review request for Aurora and Kevin

Re: Review Request 26123: Fail the build on lack of test coverage.

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26123/#review54902 --- Ship it! Ship It! - Kevin Sweeney On Sept. 29, 2014, 2:10 p.m.,

Re: Review Request 26154: Stop sending and receiving DeletedTasks framework message.

2014-09-29 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26154/#review54903 --- src/main/python/apache/aurora/executor/gc_executor.py

Re: Review Request 26102: Dropping assert on empty desired instances set.

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

Re: Review Request 26123: Fail the build on lack of test coverage.

2014-09-29 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26123/#review54908 --- Ship it! Thanks for clarifying the assert message. - Joshua Cohen

Re: Review Request 26102: Dropping assert on empty desired instances set.

2014-09-29 Thread Maxim Khutornenko
On Sept. 29, 2014, 8:12 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java, line 88 https://reviews.apache.org/r/26102/diff/2/?file=708338#file708338line88 Your call on whether this makes things more readable, but i could imagine some cleanup

Re: Review Request 26102: Dropping assert on empty desired instances set.

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

Re: Review Request 25835: Serve HTTP assets out of a standard classpath root.

2014-09-29 Thread Kevin Sweeney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25835/#review54915 --- Ship it! Ship It! - Kevin Sweeney On Sept. 19, 2014, 2:39 p.m.,

Re: Review Request 26102: Dropping assert on empty desired instances set.

2014-09-29 Thread Maxim Khutornenko
On Sept. 29, 2014, 11:10 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java, line 197 https://reviews.apache.org/r/26102/diff/3/?file=708816#file708816line197 Adding optional here seems circuitous. Why not avoid calling `asMap` if `config` is

Re: Review Request 26102: Dropping assert on empty desired instances set.

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

Re: Review Request 26123: Fail the build on lack of test coverage.

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

Re: Review Request 26102: Dropping assert on empty desired instances set.

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

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Brian Wickman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review54916 --- src/main/python/apache/aurora/executor/common/announcer.py

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Sept. 29, 2014, 5:54 p.m.) Review request for Aurora, Kevin Sweeney,

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

2014-09-29 Thread Zameer Manji
On Sept. 29, 2014, 4:43 p.m., Brian Wickman wrote: src/main/python/apache/aurora/executor/common/announcer.py, lines 112-116 https://reviews.apache.org/r/25974/diff/4/?file=708538#file708538line112 this data is available within the mesos_task struct created in from_assigned_task.

Review Request 26158: Perform job update no-op detection in SchedulerThriftInterface.

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

Re: Review Request 23330: replace 143 kB favicon with 318 byte version

2014-09-29 Thread Brian Wickman
On Sept. 29, 2014, 4:31 p.m., Brian Wickman wrote: david -- could you publish a remote branch somewhere that i can use to merge this to master? the reviewboard diff does not contain the .ico and ./rbt patch does not support binary diffs. David Robinson wrote:

Re: Review Request 23330: replace 143 kB favicon with 318 byte version

2014-09-29 Thread Brian Wickman
On Sept. 29, 2014, 4:31 p.m., Brian Wickman wrote: david -- could you publish a remote branch somewhere that i can use to merge this to master? the reviewboard diff does not contain the .ico and ./rbt patch does not support binary diffs. David Robinson wrote:

Review Request 26160: Fixing consumption double counting in quota checks.

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

Re: Review Request 26158: Perform job update no-op detection in SchedulerThriftInterface.

2014-09-29 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26158/#review54934 --- Not quite obvious from the diff: why was not it enough to check the

Re: Review Request 26154: Stop sending and receiving DeletedTasks framework message.

2014-09-29 Thread Bill Farner
On Sept. 29, 2014, 10:51 p.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/executor/gc_executor.py, line 439 https://reviews.apache.org/r/26154/diff/1/?file=708560#file708560line439 Should the reconcile_states() be adjusted as well to not return remote_gc set? Seems

Re: Review Request 26160: Fixing consumption double counting in quota checks.

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

Re: Review Request 26154: Stop sending and receiving DeletedTasks framework message.

2014-09-29 Thread Bill Farner
On Sept. 29, 2014, 10:26 p.m., Kevin Sweeney wrote: src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java, line 21 https://reviews.apache.org/r/26154/diff/1/?file=708562#file708562line21 Prefer

Re: Review Request 26154: Stop sending and receiving DeletedTasks framework message.

2014-09-29 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26154/ --- (Updated Sept. 30, 2014, 3:36 a.m.) Review request for Aurora, Maxim