Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187560
---



Master (2df250e) is red with this patch.
  ./build-support/jenkins/build.sh

 
.pants.d/python-setup/chroots/ba32407587069aafa8e32c641525ba88361a07d7/.deps/mock-1.0.1-py2-none-any.whl/mock.py:1201:
 
 _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
 
 mocks = (, 
, , , )
 td = '/tmp/tmpmUI8jr'
 some_user = pwd.struct_passwd(pw_name='jglick', 
pw_passwd='*', pw_uid=1939, pw_gid=1939, pw_gecos='Jesse N. Glick', 
pw_dir='/home/jglick', pw_shell='/usr/local/bin/bash')
 taskpath = 
 
 @mock.patch('os.chown')
 @mock.patch('os.setgroups')
 @mock.patch('os.setgid')
 @mock.patch('os.setuid')
 @mock.patch('os.geteuid', return_value=0)
 def test_log_permissions_other_user(*mocks):
   with temporary_dir() as td:
 some_user = get_other_nonroot_user()
 taskpath = make_taskpath(td)
 sandbox = setup_sandbox(td, taskpath)
 
 p = TestProcess('process', 'echo hello world', 
0, taskpath, sandbox, user=some_user.pw_name)
 p.start()
 
wait_for_rc(taskpath.getpath('process_checkpoint'))
 
 # since we're not actually root, the best we 
can do is check the right things were attempted
 stdout = 
taskpath.with_filename('stdout').getpath('process_logdir')
 stderr = 
taskpath.with_filename('stderr').getpath('process_logdir')
 >   assert os.path.exists(stdout)
 E   assert ('/tmp/tmpmUI8jr/.logs/process/0/stdout')
 E+  where  = .exists
 E+where  = os.path
 
 src/test/python/apache/thermos/core/test_process.py:264: 
AssertionError
  generated xml file: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/aaf4d108c31293299a0839bdc404a91802f80937.xml
 
  1 failed, 795 passed, 6 skipped, 1 warnings in 
318.42 seconds 
 
FAILURE


17:35:22 05:54   [complete]
   FAILURE


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 10, 2017, 5:02 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 10, 2017, 5:02 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 84a6d37d8a643047f97a03187fae1b3616633650 
>   ui/package.json 6e8ad7a3f7a5d6c450ded4e2c1f216cfa4551a2c 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 50d63e6252b38cf26688ab032409a00082561f14 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread Santhosh Kumar Shanmugham 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187558
---


Ship it!




Ship It!

- Santhosh Kumar Shanmugham


On Oct. 10, 2017, 10:02 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 10, 2017, 10:02 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 84a6d37d8a643047f97a03187fae1b3616633650 
>   ui/package.json 6e8ad7a3f7a5d6c450ded4e2c1f216cfa4551a2c 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 50d63e6252b38cf26688ab032409a00082561f14 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 8f077341b2194fc00d202b0eb289b23986d0f2e0 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/Updates.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
>   ui/src/main/js/utils/Common.js be8766c3b53943dcf094670019ec528975dc05b5 
>   ui/src/main/js/utils/Thrift.js b247e36dfa7e8b686e8e6caccfec9ff813829bab 
>   ui/src/main/js/utils/Update.js PRE-CREATION 
>   ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/sass/app.scss e301d4c31baed10faf06d84f30d1c1dbce66952a 
>   ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
>   ui/src/main/sass/components/_layout.scss 
> 4ebec0532550a0c6084fb603cfab5789c1ccb371 
>   ui/src/main/sass/components/_update-list.scss PRE-CREATION 
>   ui/src/main/sass/components/_update-page.scss PRE-CREATION 
>   ui/test-setup.js a403434ddd7d6b33b7e18ca8c25ab68707a35fd8 
> 
> 
> Diff: https://reviews.apache.org/r/62763/diff/3/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> Tested against live APIs. Note: I am pushing small nitpick bugs to the very 
> end of this work (where we will leverage internal beta-testing).
> 
> 
> File Attachments
> 
> 
> Successful update
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
> Update In-Progress
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
> Huge update
>   
>

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread David McLaughlin 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/
---

(Updated Oct. 10, 2017, 5:02 p.m.)


Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Changes
---

feedback.


Repository: aurora


Description
---

Implement update page for new UI. 

The most significant change here is the performance - for our jobs at Twitter 
with thousands of instances, this shaves tens of seconds off the render time 
compared to the Angular version (which I think is attributed to the tooltip 
code). 

I translated the "instance events" into something useful - now grouping them 
all events for an instance together in a single item and showing history with 
the state machine component. 

For the instance visualization, I also simplified the color scheme. There is 
now only: success, error, warning, in progress and removed. So no more roll 
backs being represented as green boxes with red borders (roll backs are now 
considered as the warning/attention color). 

I didn't do much for configuration and kept the UX the same as the existing UI, 
I will tackle showing diffs in the future. 

This is branched off of https://reviews.apache.org/r/62720


Diffs (updated)
-

  ui/.eslintrc 84a6d37d8a643047f97a03187fae1b3616633650 
  ui/package.json 6e8ad7a3f7a5d6c450ded4e2c1f216cfa4551a2c 
  ui/src/main/js/components/InstanceViz.js PRE-CREATION 
  ui/src/main/js/components/Layout.js 50d63e6252b38cf26688ab032409a00082561f14 
  ui/src/main/js/components/Pagination.js 
7bf2c04e2b879657c6ec43d261f2512fae79a08e 
  ui/src/main/js/components/TaskConfig.js PRE-CREATION 
  ui/src/main/js/components/Time.js PRE-CREATION 
  ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
  ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
  ui/src/main/js/components/UpdateList.js PRE-CREATION 
  ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
  ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
  ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
  ui/src/main/js/components/UpdateTime.js PRE-CREATION 
  ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
  ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/Pagination-test.js 
f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
  ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
  ui/src/main/js/index.js 8f077341b2194fc00d202b0eb289b23986d0f2e0 
  ui/src/main/js/pages/Update.js PRE-CREATION 
  ui/src/main/js/pages/Updates.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
  ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
  ui/src/main/js/utils/Common.js be8766c3b53943dcf094670019ec528975dc05b5 
  ui/src/main/js/utils/Thrift.js b247e36dfa7e8b686e8e6caccfec9ff813829bab 
  ui/src/main/js/utils/Update.js PRE-CREATION 
  ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/sass/app.scss e301d4c31baed10faf06d84f30d1c1dbce66952a 
  ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
  ui/src/main/sass/components/_layout.scss 
4ebec0532550a0c6084fb603cfab5789c1ccb371 
  ui/src/main/sass/components/_update-list.scss PRE-CREATION 
  ui/src/main/sass/components/_update-page.scss PRE-CREATION 
  ui/test-setup.js a403434ddd7d6b33b7e18ca8c25ab68707a35fd8 


Diff: https://reviews.apache.org/r/62763/diff/3/

Changes: https://reviews.apache.org/r/62763/diff/2-3/


Testing
---

./gradlew ui:lint
./gradlew ui:test

Tested against live APIs. Note: I am pushing small nitpick bugs to the very end 
of this work (where we will leverage internal beta-testing).


File Attachments


Successful update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
Update In-Progress
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
Huge update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/5b2534e3-e5d6-48e8-9615-2b1f7bb13617__Screen_Shot_2017-10-04_at_5.27.15_PM.png


Thanks,

David McLaughlin

Re: Review Request 62763: Create Update Page with React

2017-10-10 Thread David McLaughlin 


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/__tests__/InstanceViz-test.js
> > Lines 23 (patched)
> > 
> >
> > nit - assert `medium` and `big` do not appear?

Done.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/__tests__/Pagination-test.js
> > Lines 78 (patched)
> > 
> >
> > We do not sort when `sortBy` is omitted. What do you mean by native 
> > sort?

The natural order of the collection (i.e. order by array index). Updated 
description.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/utils/__tests__/Update-test.js
> > Lines 47 (patched)
> > 
> >
> > s/success/inprogress/

Fixed.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/client/scheduler-client.js
> > Line 2 (original), 2 (patched)
> > 
> >
> > Drop this?

Fixed.


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > ui/src/main/js/components/UpdateInstanceEvents.js
> > Lines 50 (patched)
> > 
> >
> > The `events` that are pushed into this component are already sorted. Do 
> > we require an extra sort?

In the parent component, they are *reverse* sorted (to show a descending list 
of latest instances updated) and then here they are sorted in ascending order 
(for the state machine).


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187281
---


On Oct. 5, 2017, 10:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 10:44 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>

Re: Review Request 62763: Create Update Page with React

2017-10-06 Thread Santhosh Kumar Shanmugham 


> On Oct. 6, 2017, 12:46 p.m., Santhosh Kumar Shanmugham wrote:
> > Looks good to me overall.
> > 
> > I see the pattern that we are passing in the entire `update` object to all 
> > the components, when they only consume a small part of it. Any reason for 
> > keeping it this way.
> 
> David McLaughlin wrote:
> This is mostly related to the plugin architecture. I'd rather be liberal 
> so that if people want to drop in customizations, they have access to as much 
> data as possible in the components they swap out. 
> 
> But in general I also prefer to not destructure the objects all over the 
> client - it makes it harder to change later if the API changes.

Cool. Those make sense.


- Santhosh Kumar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187281
---


On Oct. 5, 2017, 3:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 3:44 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/Updates.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
>   ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
>   ui/src/main/js/utils/Thrift.js PRE-CREATION 
>   ui/src/main/js/utils/Update.js PRE-CREATION 
>   ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
>   ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
>   ui/src/main/sass/components/_layout.scss 
> 1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
>   ui/src/main/sass/components/_update-list.scss PRE-CREATION 
>   ui/src/main/sass/components/_update-page.scss PRE-CREATION 
>   ui/test-setup.js

Re: Review Request 62763: Create Update Page with React

2017-10-06 Thread David McLaughlin 


> On Oct. 6, 2017, 7:46 p.m., Santhosh Kumar Shanmugham wrote:
> > Looks good to me overall.
> > 
> > I see the pattern that we are passing in the entire `update` object to all 
> > the components, when they only consume a small part of it. Any reason for 
> > keeping it this way.

This is mostly related to the plugin architecture. I'd rather be liberal so 
that if people want to drop in customizations, they have access to as much data 
as possible in the components they swap out. 

But in general I also prefer to not destructure the objects all over the client 
- it makes it harder to change later if the API changes.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187281
---


On Oct. 5, 2017, 10:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 10:44 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/Updates.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
>   ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
>   ui/src/main/js/utils/Thrift.js PRE-CREATION 
>   ui/src/main/js/utils/Update.js PRE-CREATION 
>   ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
>   ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
>   ui/src/main/sass/components/_layout.scss 
> 1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
>   ui/src/main/sass/components/_update-list.scss PRE-CREATION 
>   ui/src/main/sass/components/_update-page.scss PRE-CREATION 
>   ui/test-setup.js 054e7c2302aa70bc94a18581af5bd8e4e70559b1 
> 
> 
> Diff: https://reviews.apache.org/r/62763/diff/2/
> 
> 
>

Re: Review Request 62763: Create Update Page with React

2017-10-06 Thread Santhosh Kumar Shanmugham 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187281
---


Fix it, then Ship it!




Looks good to me overall.

I see the pattern that we are passing in the entire `update` object to all the 
components, when they only consume a small part of it. Any reason for keeping 
it this way.


ui/src/main/js/components/UpdateSettings.js
Lines 5 (patched)


Wonder if we should just pass in the `settings`? This seems to be a 
prevalent pattern across the code.



ui/src/main/js/components/__tests__/InstanceViz-test.js
Lines 23 (patched)


nit - assert `medium` and `big` do not appear?



ui/src/main/js/components/__tests__/Pagination-test.js
Lines 78 (patched)


We do not sort when `sortBy` is omitted. What do you mean by native sort?



ui/src/main/js/utils/__tests__/Update-test.js
Lines 47 (patched)


s/success/inprogress/



ui/src/main/js/client/scheduler-client.js
Line 2 (original), 2 (patched)


Drop this?



ui/src/main/js/components/UpdateInstanceEvents.js
Lines 50 (patched)


The `events` that are pushed into this component are already sorted. Do we 
require an extra sort?


- Santhosh Kumar Shanmugham


On Oct. 5, 2017, 3:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 3:44 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/Updates.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
>

Re: Review Request 62763: Create Update Page with React

2017-10-05 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/#review187215
---



This patch does not apply cleanly against master (0f1e684), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 5, 2017, 10:44 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 10:44 p.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateList.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/Updates.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
>   ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
>   ui/src/main/js/utils/Thrift.js PRE-CREATION 
>   ui/src/main/js/utils/Update.js PRE-CREATION 
>   ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
>   ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
>   ui/src/main/sass/components/_layout.scss 
> 1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
>   ui/src/main/sass/components/_update-list.scss PRE-CREATION 
>   ui/src/main/sass/components/_update-page.scss PRE-CREATION 
>   ui/test-setup.js 054e7c2302aa70bc94a18581af5bd8e4e70559b1 
> 
> 
> Diff: https://reviews.apache.org/r/62763/diff/2/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> Tested against live APIs. Note: I am pushing small nitpick bugs to the very 
> end of this work (where we will leverage internal beta-testing).
> 
> 
> File Attachments
> 
> 
> Successful update
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
> Update In-Progress
>   
>

Re: Review Request 62763: Create Update Page with React

2017-10-05 Thread David McLaughlin 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62763/
---

(Updated Oct. 5, 2017, 10:44 p.m.)


Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.


Changes
---

Add /updates to this review too.


Repository: aurora


Description
---

Implement update page for new UI. 

The most significant change here is the performance - for our jobs at Twitter 
with thousands of instances, this shaves tens of seconds off the render time 
compared to the Angular version (which I think is attributed to the tooltip 
code). 

I translated the "instance events" into something useful - now grouping them 
all events for an instance together in a single item and showing history with 
the state machine component. 

For the instance visualization, I also simplified the color scheme. There is 
now only: success, error, warning, in progress and removed. So no more roll 
backs being represented as green boxes with red borders (roll backs are now 
considered as the warning/attention color). 

I didn't do much for configuration and kept the UX the same as the existing UI, 
I will tackle showing diffs in the future. 

This is branched off of https://reviews.apache.org/r/62720


Diffs (updated)
-

  ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
  ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
  ui/src/main/js/client/scheduler-client.js 
1c381086934dafa22a62d18e035f599fb2260e15 
  ui/src/main/js/components/InstanceViz.js PRE-CREATION 
  ui/src/main/js/components/Layout.js 4ca54e318786859e23dc0444d7d2750817428e81 
  ui/src/main/js/components/Pagination.js 
7bf2c04e2b879657c6ec43d261f2512fae79a08e 
  ui/src/main/js/components/TaskConfig.js PRE-CREATION 
  ui/src/main/js/components/Time.js PRE-CREATION 
  ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
  ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
  ui/src/main/js/components/UpdateList.js PRE-CREATION 
  ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
  ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
  ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
  ui/src/main/js/components/UpdateTime.js PRE-CREATION 
  ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
  ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/Pagination-test.js 
f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
  ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateList-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
  ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
  ui/src/main/js/pages/Update.js PRE-CREATION 
  ui/src/main/js/pages/Updates.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Updates-test.js PRE-CREATION 
  ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
  ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
  ui/src/main/js/utils/Thrift.js PRE-CREATION 
  ui/src/main/js/utils/Update.js PRE-CREATION 
  ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
  ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
  ui/src/main/sass/components/_layout.scss 
1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
  ui/src/main/sass/components/_update-list.scss PRE-CREATION 
  ui/src/main/sass/components/_update-page.scss PRE-CREATION 
  ui/test-setup.js 054e7c2302aa70bc94a18581af5bd8e4e70559b1 


Diff: https://reviews.apache.org/r/62763/diff/2/

Changes: https://reviews.apache.org/r/62763/diff/1-2/


Testing
---

./gradlew ui:lint
./gradlew ui:test

Tested against live APIs. Note: I am pushing small nitpick bugs to the very end 
of this work (where we will leverage internal beta-testing).


File Attachments


Successful update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
Update In-Progress
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
Huge update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/5b2534e3-e5d6-48e8-9615-2b1f7bb13617__Screen_Shot_2017-10-04_at_5.27.15_PM.png


Thanks,

David McLaughlin

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 you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Oct. 5, 2017, 12:34 a.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62763/
> ---
> 
> (Updated Oct. 5, 2017, 12:34 a.m.)
> 
> 
> Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Implement update page for new UI. 
> 
> The most significant change here is the performance - for our jobs at Twitter 
> with thousands of instances, this shaves tens of seconds off the render time 
> compared to the Angular version (which I think is attributed to the tooltip 
> code). 
> 
> I translated the "instance events" into something useful - now grouping them 
> all events for an instance together in a single item and showing history with 
> the state machine component. 
> 
> For the instance visualization, I also simplified the color scheme. There is 
> now only: success, error, warning, in progress and removed. So no more roll 
> backs being represented as green boxes with red borders (roll backs are now 
> considered as the warning/attention color). 
> 
> I didn't do much for configuration and kept the UX the same as the existing 
> UI, I will tackle showing diffs in the future. 
> 
> This is branched off of https://reviews.apache.org/r/62720
> 
> 
> Diffs
> -
> 
>   ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
>   ui/src/main/js/client/scheduler-client.js 
> 1c381086934dafa22a62d18e035f599fb2260e15 
>   ui/src/main/js/components/InstanceViz.js PRE-CREATION 
>   ui/src/main/js/components/Layout.js 
> 4ca54e318786859e23dc0444d7d2750817428e81 
>   ui/src/main/js/components/Pagination.js 
> 7bf2c04e2b879657c6ec43d261f2512fae79a08e 
>   ui/src/main/js/components/TaskConfig.js PRE-CREATION 
>   ui/src/main/js/components/Time.js PRE-CREATION 
>   ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
>   ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
>   ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
>   ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
>   ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTime.js PRE-CREATION 
>   ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
>   ui/src/main/js/components/__tests__/Pagination-test.js 
> f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
>   ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js 
> PRE-CREATION 
>   ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
>   ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
>   ui/src/main/js/pages/Update.js PRE-CREATION 
>   ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
>   ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
>   ui/src/main/js/utils/Thrift.js PRE-CREATION 
>   ui/src/main/js/utils/Update.js PRE-CREATION 
>   ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
>   ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
>   ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
>   ui/src/main/sass/components/_layout.scss 
> 1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
>   ui/src/main/sass/components/_update-page.scss PRE-CREATION 
>   ui/test-setup.js 054e7c2302aa70bc94a18581af5bd8e4e70559b1 
> 
> 
> Diff: https://reviews.apache.org/r/62763/diff/1/
> 
> 
> Testing
> ---
> 
> ./gradlew ui:lint
> ./gradlew ui:test
> 
> Tested against live APIs. Note: I am pushing small nitpick bugs to the very 
> end of this work (where we will leverage internal beta-testing).
> 
> 
> File Attachments
> 
> 
> Successful update
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
> Update In-Progress
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
> Huge update
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/05/5b2534e3-e5d6-48e8-9615-2b1f7bb13617__Screen_Shot_2017-10-04_at_5.27.15_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>

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.


Repository: aurora


Description
---

Implement update page for new UI. 

The most significant change here is the performance - for our jobs at Twitter 
with thousands of instances, this shaves tens of seconds off the render time 
compared to the Angular version (which I think is attributed to the tooltip 
code). 

I translated the "instance events" into something useful - now grouping them 
all events for an instance together in a single item and showing history with 
the state machine component. 

For the instance visualization, I also simplified the color scheme. There is 
now only: success, error, warning, in progress and removed. So no more roll 
backs being represented as green boxes with red borders (roll backs are now 
considered as the warning/attention color). 

I didn't do much for configuration and kept the UX the same as the existing UI, 
I will tackle showing diffs in the future. 

This is branched off of https://reviews.apache.org/r/62720


Diffs
-

  ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
  ui/src/main/js/client/scheduler-client.js 
1c381086934dafa22a62d18e035f599fb2260e15 
  ui/src/main/js/components/InstanceViz.js PRE-CREATION 
  ui/src/main/js/components/Layout.js 4ca54e318786859e23dc0444d7d2750817428e81 
  ui/src/main/js/components/Pagination.js 
7bf2c04e2b879657c6ec43d261f2512fae79a08e 
  ui/src/main/js/components/TaskConfig.js PRE-CREATION 
  ui/src/main/js/components/Time.js PRE-CREATION 
  ui/src/main/js/components/UpdateConfig.js PRE-CREATION 
  ui/src/main/js/components/UpdateDetails.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceEvents.js PRE-CREATION 
  ui/src/main/js/components/UpdateInstanceSummary.js PRE-CREATION 
  ui/src/main/js/components/UpdateSettings.js PRE-CREATION 
  ui/src/main/js/components/UpdateStateMachine.js PRE-CREATION 
  ui/src/main/js/components/UpdateStatus.js PRE-CREATION 
  ui/src/main/js/components/UpdateTime.js PRE-CREATION 
  ui/src/main/js/components/UpdateTitle.js PRE-CREATION 
  ui/src/main/js/components/__tests__/InstanceViz-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/Pagination-test.js 
f2b72e93eabe9660f3ca54d3e151ce154fa81e0a 
  ui/src/main/js/components/__tests__/UpdateInstanceEvents-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/UpdateStatus-test.js PRE-CREATION 
  ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
  ui/src/main/js/pages/Update.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/js/test-utils/UpdateBuilders.js PRE-CREATION 
  ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
  ui/src/main/js/utils/Thrift.js PRE-CREATION 
  ui/src/main/js/utils/Update.js PRE-CREATION 
  ui/src/main/js/utils/__tests__/Update-test.js PRE-CREATION 
  ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
  ui/src/main/sass/components/_instance-viz.scss PRE-CREATION 
  ui/src/main/sass/components/_layout.scss 
1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
  ui/src/main/sass/components/_update-page.scss PRE-CREATION 
  ui/test-setup.js 054e7c2302aa70bc94a18581af5bd8e4e70559b1 


Diff: https://reviews.apache.org/r/62763/diff/1/


Testing
---

./gradlew ui:lint
./gradlew ui:test

Tested against live APIs. Note: I am pushing small nitpick bugs to the very end 
of this work (where we will leverage internal beta-testing).


File Attachments


Successful update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/457b8fe0-dfd4-4e30-bbd3-e5c76906fab2__Screen_Shot_2017-10-04_at_5.22.08_PM.png
Update In-Progress
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/d04afb4c-7110-4fe2-a0d9-c191b935a555__Screen_Shot_2017-10-04_at_5.25.40_PM.png
Huge update
  
https://reviews.apache.org/media/uploaded/files/2017/10/05/5b2534e3-e5d6-48e8-9615-2b1f7bb13617__Screen_Shot_2017-10-04_at_5.27.15_PM.png


Thanks,

David McLaughlin