Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186421 --- > Unfortunately the Gradle 4.2 upgrade seems to have broken

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186422 --- Ship it! Master (474320b) is green with this patch.

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Bill Farner
> On Sept. 27, 2017, 9:08 a.m., Bill Farner wrote: > > > Unfortunately the Gradle 4.2 upgrade seems to have broken running builds > > > from gradle > > > > I didn't witness 4.2 causing breakage, but did find other issues, patched > > at [62620](https://reviews.apache.org/r/62620/). I'll be

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
> On Sept. 27, 2017, 4:28 p.m., Joshua Cohen wrote: > > FWIW there's also a version of React 15 that's been licensed under MIT: > > https://facebook.github.io/react/blog/2017/09/25/react-v15.6.2.html Ah, good to know. I think this is the correct approach anyway. Reactable would eventually

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/ --- (Updated Sept. 27, 2017, 5:25 p.m.) Review request for Aurora, Santhosh Kumar

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186435 --- Ship it! Master (474320b) is green with this patch.

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/#review186446 --- @ReviewBot retry - Jordan Ly On Sept. 27, 2017, 5:25 p.m.,

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/ --- (Updated Sept. 27, 2017, 5:51 p.m.) Review request for Aurora, Kai Huang and

Re: Review Request 62608: Workaround to get pants working in macOS high sierra

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62608/#review186427 --- @ReviewBot retry One last shot, i'm unable to reproduce the

Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/ --- Review request for Aurora, Santhosh Kumar Shanmugham, Stephan Erb, and Bill

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
> On Sept. 27, 2017, 4:08 p.m., Bill Farner wrote: > > > Unfortunately the Gradle 4.2 upgrade seems to have broken running builds > > > from gradle > > > > I didn't witness 4.2 causing breakage, but did find other issues, patched > > at [62620](https://reviews.apache.org/r/62620/). I'll be

Re: Review Request 62620: Fix binding issues preventing ./gradle run from working

2017-09-27 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62620/#review186443 --- Ship it! Ship It! - Jordan Ly On Sept. 27, 2017, 4:06 p.m.,

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/ --- (Updated Sept. 27, 2017, 4:50 p.m.) Review request for Aurora, Kai Huang and

Review Request 62620: Fix binding issues preventing ./gradle run from working

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62620/ --- Review request for Aurora and David McLaughlin. Repository: aurora

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186425 --- FWIW there's also a version of React 15 that's been licensed

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/#review186439 --- Ship it! Ship It! - Bill Farner On Sept. 27, 2017, 10:11

Re: Review Request 62608: Workaround to get pants working in macOS high sierra

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62608/#review186442 --- Ship it! Master (474320b) is green with this patch.

Re: Review Request 62620: Fix binding issues preventing ./gradle run from working

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62620/#review186444 --- Master (474320b) is red with this patch.

Re: Review Request 62620: Fix binding issues preventing ./gradle run from working

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62620/#review186456 --- Ship it! Master (474320b) is green with this patch.

Re: Review Request 62601: Remove the rewriteConfigs thrift method

2017-09-27 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62601/#review186466 --- Ship it! I love commits that are just pure removal :) Please

Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62626/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1950

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/#review186455 --- Master (474320b) is green with this patch.

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Jordan Ly
> On Sept. 27, 2017, 8:33 p.m., Reza Motamedi wrote: > > src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java > > Line 109 (original), 109 (patched) > > > > > > Is just testing shutdown of the

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186454 --- Ship it! Master (474320b) is green with this patch.

Re: Review Request 62397: Replica Hot Standby

2017-09-27 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62397/#review186461 --- I won't have time for a proper review before next week. However,

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/#review186464 --- Ship it! Ship It! - Stephan Erb On Sept. 27, 2017, 9:27

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/ --- (Updated Sept. 27, 2017, 9:27 p.m.) Review request for Aurora, Santhosh Kumar

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

2017-09-27 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) Bison on MacOs is 10 years old. I assumed they have a good

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62626/#review186473 ---

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186469 --- ui/src/main/js/components/Pagination.js Lines 75 (patched)

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186472 --- Ship it! LGTM. Minor comments about readability.

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186498 --- @ReviewBot retry - Stephan Erb On Sept. 27, 2017, 11:31 p.m.,

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62626/#review186488 --- Ship it! Wonder if this will fix the issue with shutting down

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186490 --- Master (abd6fad) is red with this patch.

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
> On Sept. 27, 2017, 8:35 p.m., Kai Huang wrote: > > ui/src/main/js/components/Pagination.js > > Lines 75 (patched) > > > > > > nit, the variable name here collide with the `currentPage` attribute in > > Pages

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186485 --- Ship it! Ship It! - Kai Huang On Sept. 27, 2017, 9:31 p.m.,

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62626/#review186497 --- Ship it! Ship It! - Stephan Erb On Sept. 27, 2017, 10:25

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62626/#review186518 --- Ship it! Ship It! - Bill Farner On Sept. 27, 2017, 1:25

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

2017-09-27 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

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

2017-09-27 Thread David McLaughlin
> On Sept. 26, 2017, 12:27 a.m., Santhosh Kumar Shanmugham wrote: > > ui/src/main/js/components/JobList.js > > Lines 97 (patched) > > > > > > It is possible for clusters to have customer tier configuration. Should >

Re: Review Request 62397: Replica Hot Standby

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62397/#review186521 --- FYI - i plan to open a discussion on this soon; but i intend to

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
> On Sept. 27, 2017, 4:08 p.m., Bill Farner wrote: > > > Unfortunately the Gradle 4.2 upgrade seems to have broken running builds > > > from gradle > > > > I didn't witness 4.2 causing breakage, but did find other issues, patched > > at [62620](https://reviews.apache.org/r/62620/). I'll be

Re: Review Request 62621: Allow transitions from any state to STOPPED in CallOrderEnforcingStorage

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62621/#review186445 --- Master (474320b) is red with this patch.

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

2017-09-27 Thread Kai Huang
> On Sept. 27, 2017, 12:15 a.m., Kai Huang wrote: > > ui/src/main/js/index.js > > Line 17 (original), 18 (patched) > > > > > > In the homepage, it seems the role link still points to the old > > role/environment

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

2017-09-27 Thread David McLaughlin
> On Sept. 25, 2017, 5:40 p.m., Kai Huang wrote: > > ui/src/main/js/components/RoleQuota.js > > Lines 70 (patched) > > > > > > From the user perspective, a pie chart for the resource utilization > > would be much

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

2017-09-27 Thread David McLaughlin
> On Sept. 27, 2017, 12:15 a.m., Kai Huang wrote: > > ui/src/main/js/index.js > > Line 17 (original), 18 (patched) > > > > > > In the homepage, it seems the role link still points to the old > > role/environment

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

2017-09-27 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/#review186511 --- Ship it! LGTM - Kai Huang On Sept. 28, 2017, 12:18 a.m.,

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

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/#review186514 --- Ship it! Master (7c78519) is green with this patch.

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Jordan Ly
> On Sept. 27, 2017, 9:45 p.m., Santhosh Kumar Shanmugham wrote: > > Wonder if this will fix the issue with shutting down the Webhooks > > AsyncHttpClient as well? This should just always call the shutdown sequence when `run()` ends, (ie. `stop()` is always called in the case of errors and

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

2017-09-27 Thread David McLaughlin
> On Sept. 26, 2017, 12:27 a.m., Santhosh Kumar Shanmugham wrote: > > ui/src/main/js/components/JobList.js > > Lines 15 (patched) > > > > > > nit - Can we use `k.replace('TaskCount', '')` here as well like below?

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

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/ --- (Updated Sept. 28, 2017, 12:17 a.m.) Review request for Aurora, Joshua Cohen,

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

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/ --- (Updated Sept. 28, 2017, 12:18 a.m.) Review request for Aurora, Joshua Cohen,

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

2017-09-27 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62451/#review186513 --- Ship it! I just picked up a little bit of js. LGTM. - Reza

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

2017-09-27 Thread Santhosh Kumar Shanmugham
> On Sept. 25, 2017, 5:27 p.m., Santhosh Kumar Shanmugham wrote: > > ui/src/main/js/components/JobList.js > > Lines 97 (patched) > > > > > > It is possible for clusters to have customer tier configuration. Should >

Re: Review Request 62626: Added additional stop() to prevent errors in run() to stop shutdown in SchedulerMain

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62626/#review186478 --- Ship it! Master (abd6fad) is green with this patch.

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
> On Sept. 27, 2017, 8:59 p.m., Santhosh Kumar Shanmugham wrote: > > ui/src/main/js/components/Pagination.js > > Lines 3 (patched) > > > > > > nit - `maxPages` and `numPages` are a little confusing. Can we use the >

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/ --- (Updated Sept. 27, 2017, 9:31 p.m.) Review request for Aurora, Kai Huang and

Review Request 62652: Remove legacy commons ZK code

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62652/ --- Review request for Aurora, David McLaughlin and John Sirois. Repository:

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

2017-09-27 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62623/#review186527 --- Master (7c78519) is red with this patch.

Re: Review Request 62652: Remove legacy commons ZK code

2017-09-27 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62652/#review186522 --- -1. Please see here for details on the current status of

Re: Review Request 62607: Replace Preact and custom testing with React + Enzyme

2017-09-27 Thread Joshua Cohen
> On Sept. 27, 2017, 8:35 p.m., Kai Huang wrote: > > ui/src/main/js/components/Pagination.js > > Lines 93 (patched) > > > > > > Should this be extracted into renderer? So that we don't need to pass > > isTable

Review Request 62623: Use a simpler command line argument system

2017-09-27 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62623/ --- Review request for Aurora, David McLaughlin and John Sirois. Repository:

Review Request 62607: Replace Preact and custom testing with React + Enzyme

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