aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David McLaughlin <da...@dmclaughlin.com>
Subject Review Request 62720: Implement Instance pages in React
Date Wed, 04 Oct 2017 05:34:49 GMT

-----------------------------------------------------------
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: aurora


Description
-------

Instance page moved over to the new UI. 

One of the big issues I ran into here was dealing with Thrift types. Currently the Thrift
compiler generates a file for use in JS. The compiler gives you two options for JavaScript
- one is 'node' which plays well with our module system but assumes you'll be using the built-in
socket implementation in node, and then one for 'jquery' which uses the global namespace but
doesn't use vars (so we can't eval as it breaks strict mode). So the TL;DR is - to write tests
against scheduler states, I've had to copy and paste the Thrift enums into a test setup file.


The alternative is to simply ignore Thrift types and duplicate it all without relying on globals.
Thus making sure the tests and application code are using the same enums. However, this approach
won't help you find any UI regressions when you update Thrift - and even worse, you won't
spot any API regressions when developing the UI. So I've just gone for this approach for now.


Diffs
-----

  ui/.eslintrc 8d37c60e1edd4528ce4abdf6dda18ec2dfc078f4 
  ui/package.json fe0397a888b337137c1d13b6f9559dae13698b0a 
  ui/src/main/js/client/scheduler-client.js 1c381086934dafa22a62d18e035f599fb2260e15 
  ui/src/main/js/components/InstanceHistory.js PRE-CREATION 
  ui/src/main/js/components/InstanceHistoryItem.js PRE-CREATION 
  ui/src/main/js/components/Layout.js 4ca54e318786859e23dc0444d7d2750817428e81 
  ui/src/main/js/components/StateMachine.js PRE-CREATION 
  ui/src/main/js/components/TaskDetails.js PRE-CREATION 
  ui/src/main/js/components/TaskStatus.js PRE-CREATION 
  ui/src/main/js/components/__tests__/InstanceHistory-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/InstanceHistoryItem-test.js PRE-CREATION 
  ui/src/main/js/components/__tests__/StateMachine-test.js PRE-CREATION 
  ui/src/main/js/index.js 4a879e6163eaabc203f2d3133419438c4e9730e8 
  ui/src/main/js/pages/Instance.js PRE-CREATION 
  ui/src/main/js/pages/__tests__/Instance-test.js PRE-CREATION 
  ui/src/main/js/test-utils/Builder.js PRE-CREATION 
  ui/src/main/js/test-utils/TaskBuilders.js PRE-CREATION 
  ui/src/main/js/test-utils/__tests__/Builder-test.js PRE-CREATION 
  ui/src/main/js/utils/Common.js 2e12e3cc6af75a62de9f11797f89dbf3279d2ce6 
  ui/src/main/js/utils/Task.js PRE-CREATION 
  ui/src/main/js/utils/Thrift.js PRE-CREATION 
  ui/src/main/sass/app.scss d9673d0e8faa0b565f92e068ed4e01de0c789e8d 
  ui/src/main/sass/components/_instance-page.scss PRE-CREATION 
  ui/src/main/sass/components/_job-list-page.scss d31344d8eabb41114f4ff4678ff841119dfdac82

  ui/src/main/sass/components/_layout.scss 1d0553b8fdee2c9e11727831b7a112f534ce3ec6 
  ui/src/main/sass/components/_state-machine.scss PRE-CREATION 
  ui/src/main/sass/components/_status.scss PRE-CREATION 
  ui/test-setup.js 054e7c2302aa70bc94a18581af5bd8e4e70559b1 


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


Testing
-------

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

Testing in Vagrant.


File Attachments
----------------

Active Task with State Machine component
  https://reviews.apache.org/media/uploaded/files/2017/10/04/16f93047-a94b-4ffa-92f2-7fc27dbed9c9__Screen_Shot_2017-10-03_at_10.12.34_PM.png
Expanded Finished Task
  https://reviews.apache.org/media/uploaded/files/2017/10/04/f5daaf35-4e4b-4106-b7ce-2773b97d2277__Screen_Shot_2017-10-03_at_10.28.18_PM.png
Tooltip
  https://reviews.apache.org/media/uploaded/files/2017/10/04/0fe652c3-48a3-4292-8179-7d09bfbcc1e4__Screen_Shot_2017-10-03_at_10.28.33_PM.png


Thanks,

David McLaughlin


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message