Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176716 --- Ship it! Ship It! - David McLaughlin On June 2, 2017, 12:01

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176711 --- Master (e76862a) is green with this patch.

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59733/#review176708 --- Ship it! Master (e76862a) is green with this patch.

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/ --- (Updated June 2, 2017, 12:01 a.m.) Review request for Aurora, David

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-01 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59733/#review176707 --- Can you add a new end-to-end test that exercises this feature?

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-01 Thread Jordan Ly
> On June 1, 2017, 11:53 p.m., David McLaughlin wrote: > > src/main/python/apache/aurora/executor/http_lifecycle.py > > Lines 37 (patched) > > > > > > Will this break for tasks that were deployed before the client

Re: Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59733/#review176705 --- src/main/python/apache/aurora/executor/http_lifecycle.py Lines

Review Request 59733: Adding Configurable Wait Period for Graceful Shutdowns

2017-06-01 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59733/ --- Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, Stephan

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/ --- (Updated June 1, 2017, 11:48 p.m.) Review request for Aurora, David

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 10:46 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Line 376 (original), 377 (patched) > > > > > > I wonder if it's using the taskIds

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
> On June 1, 2017, 10:46 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Line 376 (original), 377 (patched) > > > > > > I wonder if it's using the taskIds

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176695 --- Master (e76862a) is red with this patch.

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176694 --- This LGTM now! Just one minor thing.

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
> On June 1, 2017, 9:22 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Lines 389-391 (original), 374-376 (patched) > > > > > > We probably don't even

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/ --- (Updated June 1, 2017, 10:41 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/#review176682 --- Ship it! Ship It! - Reza Motamedi On June 1, 2017, 6:33

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 9:22 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Lines 389-391 (original), 374-376 (patched) > > > > > > We probably don't even

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/#review176681 --- src/main/java/org/apache/aurora/scheduler/events/Webhook.java

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
> On June 1, 2017, 9:22 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java > > Lines 389-391 (original), 374-376 (patched) > > > > > > We probably don't even

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176673 --- Master (e76862a) is green with this patch.

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 9:15 p.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/events/Webhook.java > > Lines 111-113 (patched) > > > > > > What are the implications of a task failing to change? Not

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/#review176670 ---

Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 9:15 p.m., Reza Motamedi wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java > > Lines 121-123 (patched) > > > > > > So, these installed modules are just needed by the

Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 8:52 p.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java > > Lines 121-123 (patched) > > > > > > Maybe move to configure It can't be in the PrivateModule

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/#review17 --- LGTM, one small question for my own understanding

Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-01 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59698/#review176660 --- Fix it, then Ship it! Getting used to Guice here... but

Re: Review Request 59699: Improve task history pruning by batch deleting tasks

2017-06-01 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59699/ --- (Updated June 1, 2017, 9:13 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 59698: Allow custom OfferManager ordering to be injected via Guice modules

2017-06-01 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59698/#review176658 --- Fix it, then Ship it! Small refactor but otherwise LGTM

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread David McLaughlin
> On June 1, 2017, 5:35 p.m., Zameer Manji wrote: > > build.gradle > > Lines 121 (patched) > > > > > > I don't see the code use netty, so why do we need this? The async-http-client uses netty, and gradle was

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/#review176631 --- otherwise this LGTM. build.gradle Lines 121 (patched)

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/#review176627 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On May 31,

Re: Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/#review176584 --- Ship it! Master (e76862a) is green with this patch.

Review Request 59703: Use async HTTP for Web Hooks.

2017-06-01 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59703/ --- Review request for Aurora, Santhosh Kumar Shanmugham, Stephan Erb, and Zameer