Re: Review Request 62720: Implement Instance pages in React

2017-10-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187052 --- Ship it! Master (4e7cdc4) is green with this patch. ./build-s

Re: Review Request 62720: Implement Instance pages in React

2017-10-03 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/ --- (Updated Oct. 4, 2017, 5:43 a.m.) Review request for Aurora, Kai Huang and Sant

Review Request 62720: Implement Instance pages in React

2017-10-03 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/ --- Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham. Repository:

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187049 --- Ship it! Master (4e7cdc4) is green with this patch. ./build-s

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187047 --- Master (4e7cdc4) is red with this patch. ./build-support/jenkins

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/ --- (Updated Oct. 4, 2017, 4:31 a.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 62623: Use a simpler command line argument system

2017-10-03 Thread David McLaughlin
> On Sept. 29, 2017, 6:17 p.m., David McLaughlin wrote: > > I'm also +1 to the spirit of the patch. One requirement to keep in mind > > beyond grouping arguments into logical modules is that we also need to make > > sure we keep the ability to add custom CLI arguments via the guice > > injecti

Re: Review Request 62623: Use a simpler command line argument system

2017-10-03 Thread Bill Farner
> On Oct. 3, 2017, 3:29 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java > > Line 106 (original), 110 (patched) > > > > > > Is Guice doing the injectio

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-03 Thread Bill Farner
> On Sept. 26, 2017, 4:53 p.m., Bill Farner wrote: > > ``` > > /bin/sh: cmake: command not found > > ``` > > > > But now i need to install cmake, so i'm not sure this pays off. > > Bill Farner wrote: > (this = the switch to cmake) > > Stephan Erb wrote: > Bison on MacOs is 10 years old

Re: Review Request 62652: Remove legacy commons ZK code

2017-10-03 Thread Bill Farner
> On Sept. 27, 2017, 8:22 p.m., David McLaughlin wrote: > > -1. > > > > Please see here for details on the current status of curator in production: > > https://issues.apache.org/jira/browse/AURORA-1840 > > Bill Farner wrote: > Aha, there was no trace of this in the code, thanks for the po

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187034 --- Ship it! Master (4e7cdc4) is green with this patch. ./build-s

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187031 --- src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/ --- (Updated Oct. 4, 2017, 12:16 a.m.) Review request for Aurora, David McLaughlin,

Re: Review Request 62590: WIP: Update to Thrift 0.10.0

2017-10-03 Thread Stephan Erb
> On Sept. 27, 2017, 1:53 a.m., Bill Farner wrote: > > ``` > > /bin/sh: cmake: command not found > > ``` > > > > But now i need to install cmake, so i'm not sure this pays off. > > Bill Farner wrote: > (this = the switch to cmake) > > Stephan Erb wrote: > Bison on MacOs is 10 years old

Re: Review Request 62652: Remove legacy commons ZK code

2017-10-03 Thread Stephan Erb
> On Sept. 28, 2017, 5:22 a.m., David McLaughlin wrote: > > -1. > > > > Please see here for details on the current status of curator in production: > > https://issues.apache.org/jira/browse/AURORA-1840 > > Bill Farner wrote: > Aha, there was no trace of this in the code, thanks for the po

Re: Review Request 62623: Use a simpler command line argument system

2017-10-03 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62623/#review187013 --- Ship it! The change looks good to me. I did not check everythin

Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-03 Thread Stephan Erb
> On Sept. 29, 2017, 7:38 p.m., Bill Farner wrote: > > src/main/java/org/apache/aurora/scheduler/app/AppModule.java > > Lines 108 (patched) > > > > > > I would find this easier to read and more flexible: > > > >

Re: Review Request 62692: Move job environment validation to the scheduler.

2017-10-03 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62692/#review187011 --- Sorry for jumping in so late. We have had discussions and some wo

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Stephan Erb
> On Oct. 3, 2017, 10:38 p.m., Bill Farner wrote: > > src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java > > Line 86 (original), 91 (patched) > > > > > > This line results in very close coupling of the

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187008 --- Ship it! The code itself looks good to me +- the switch to a pr

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187002 --- src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-10-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/#review186995 --- Ship it! Master (6fd6d50) is green with this patch. ./build-s

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review186994 --- Ship it! Ship It! - Santhosh Kumar Shanmugham On Oct. 3, 201

Re: Review Request 62451: Implement Role and Environment pages in Preact.

2017-10-03 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/ --- (Updated Oct. 3, 2017, 7:42 p.m.) Review request for Aurora, Joshua Cohen, Kai

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review186987 --- Ship it! Master (6fd6d50) is green with this patch. ./build-s

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Santhosh Kumar Shanmugham
> On Oct. 3, 2017, 10:08 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/events/Webhook.java > > Line 87 (original), 102 (patched) > > > > > > Do you know how the timeout scenario

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Jordan Ly
> On Oct. 3, 2017, 5:08 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/events/Webhook.java > > Line 87 (original), 102 (patched) > > > > > > Do you know how the timeout scenario

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review186975 --- src/main/java/org/apache/aurora/scheduler/events/Webhook.java Lin

Re: Review Request 62700: Convert Webhook to AbstractIdleService, use async HTTP client

2017-10-03 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/ --- (Updated Oct. 3, 2017, 5:01 p.m.) Review request for Aurora, David McLaughlin,