Re: Review Request 62763: Create Update Page with React

2017-10-04 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62763/#review187151 --- This patch does not apply cleanly against master (0f1e684), do

Review Request 62763: Create Update Page with React

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

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187150 --- ui/src/main/js/components/StateMachine.js Lines 37 (patched)

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Stephan Erb
> On Oct. 4, 2017, 11:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI. >

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread David McLaughlin
> On Oct. 4, 2017, 9:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI. >

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Kai Huang
> On Oct. 4, 2017, 9:01 p.m., Kai Huang wrote: > > ui/src/main/js/index.js > > Line 21 (original), 22 (patched) > > > > > > I'm not sure why I cannot access the job page and the instance page. > > Visiting this

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread David McLaughlin
> On Oct. 4, 2017, 9:42 p.m., Stephan Erb wrote: > > Regarding the question nodejs vs js thrift: nodejs supports both binary > > (good) and compact (better) thrift protocols. This should reduce scheduler > > and client side serialization overhead and lead to a slightly more snappy > > UI.

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

2017-10-04 Thread Stephan Erb
> On Oct. 4, 2017, 12:29 a.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java > > Line 106 (original), 110 (patched) > > > > > > Is Guice doing the

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread David McLaughlin
> On Oct. 4, 2017, 9:37 p.m., Stephan Erb wrote: > > ui/src/main/js/utils/Task.js > > Lines 40-41 (patched) > > > > > > Is the array index safe here? If we need the guard for > > `task.taskEvents.length` in

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187132 --- Regarding the question nodejs vs js thrift: nodejs supports both

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187128 --- Looks great! :) When playing around with the UI on the vagrant

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread David McLaughlin
> On Oct. 4, 2017, 9:01 p.m., Kai Huang wrote: > > ui/src/main/js/index.js > > Line 21 (original), 22 (patched) > > > > > > I'm not sure why I cannot access the job page and the instance page. > > Visiting this

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Kai Huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187129 --- ui/src/main/js/index.js Line 21 (original), 22 (patched)

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread David McLaughlin
> On Oct. 4, 2017, 5:50 p.m., Reza Motamedi wrote: > > ui/.eslintrc > > Lines 15 (patched) > > > > > > For my education, does this set `ACTIVE_STATES`, `Thrift`, ... up for > > you to later assign values to them?

Re: Review Request 62720: Implement Instance pages in React

2017-10-04 Thread Reza Motamedi
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62720/#review187110 --- ui/.eslintrc Lines 15 (patched)

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

2017-10-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62700/#review187094 --- Ship it! Nice! - Bill Farner On Oct. 3, 2017, 9:31 p.m.,