Re: Review Request 67696: Enable SLA-aware updates

2018-07-18 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206212 --- Ship it! Ship It! - Stephan Erb On July 18, 2018, 12:15 a.m.

Re: Review Request 67696: Enable SLA-aware updates

2018-07-18 Thread Stephan Erb
> On July 17, 2018, 10:52 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > > Lines 413-417 (patched) > > > > > > I am a bit confused about this. Can you pl

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206188 --- Ship it! Master (efe8656) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 17, 2018, 10:15 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Jordan Ly
> On July 17, 2018, 8:52 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > > Lines 413-417 (patched) > > > > > > I am a bit confused about this. Can you ple

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206173 --- Ship it! Master (efe8656) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206149 --- Looks good, thanks! I have just one question that is currently co

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 17, 2018, 6:21 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206137 --- Fix it, then Ship it! LGTM. Minor comments. src/main/java/org

Re: Review Request 67696: Enable SLA-aware updates

2018-07-16 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206143 --- Ship it! Master (a6d9288) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-16 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 16, 2018, 11 p.m.) Review request for Aurora, David McLaughlin, D

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Daniel Knightly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206034 --- Ship it! Ship It! - Daniel Knightly On July 12, 2018, 9:10 p

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Daniel Knightly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206028 --- src/main/java/org/apache/aurora/scheduler/updater/InstanceActionH

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206026 --- Ship it! Master (a6d9288) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review206024 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On July 12, 20

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Jordan Ly
> On July 9, 2018, 9:04 p.m., Santhosh Kumar Shanmugham wrote: > > src/test/java/org/apache/aurora/scheduler/updater/SlaKillControllerTest.java > > Lines 180 (patched) > > > > > > Drop `_` here and everywhere. `SLA_C

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 12, 2018, 9:10 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Santhosh Kumar Shanmugham
> On July 9, 2018, 2:04 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/updater/SlaKillController.java > > Lines 140 (patched) > > > > > > Repeating my comment from internal revie

Re: Review Request 67696: Enable SLA-aware updates

2018-07-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205973 --- Ship it! Ship It! - David McLaughlin On July 10, 2018, 2:22

Re: Review Request 67696: Enable SLA-aware updates

2018-07-10 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205843 --- docs/reference/configuration.md Lines 379 (patched)

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205886 --- Ship it! Master (a6d9288) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 10, 2018, 2:22 a.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205880 --- Master (a6d9288) is red with this patch. ./build-support/jenkins

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205879 --- @ReviewBot retry - Jordan Ly On July 9, 2018, 11:12 p.m., Jorda

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205878 --- Master (a6d9288) is red with this patch. ./build-support/jenkins

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 9, 2018, 11:12 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
> On July 9, 2018, 9:04 p.m., Santhosh Kumar Shanmugham wrote: > > end-to-end test for this feature? I think that the integration test should be sufficient for this feature (similar to coordinated updates which has fairly comprehensive integration tests but no e2e tests). > On July 9, 2018,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205866 --- end-to-end test for this feature? docs/reference/scheduler-confi

Re: Review Request 67696: Enable SLA-aware updates

2018-07-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205838 --- Ship it! Master (a6d9288) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-07 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 8, 2018, 6:48 a.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-05 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205783 --- This looks great! Very easy to read. One thing I noticed is that t

Re: Review Request 67696: Enable SLA-aware updates

2018-07-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205702 --- Ship it! Master (a6d9288) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-03 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 3, 2018, 11:29 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-07-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205697 --- Ship it! Master (a6d9288) is green with this patch. ./build-s

Re: Review Request 67696: Enable SLA-aware updates

2018-07-03 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated July 3, 2018, 6:30 p.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 67696: Enable SLA-aware updates

2018-06-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205226 --- Master (8508aec) is red with this patch. ./build-support/jenkins

Re: Review Request 67696: Enable SLA-aware updates

2018-06-21 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/ --- (Updated June 22, 2018, 12:50 a.m.) Review request for Aurora, David McLaughlin

Re: Review Request 67696: Enable SLA-aware updates

2018-06-21 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205222 --- Master (8508aec) is red with this patch. ./build-support/jenkins