Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2018-03-16 Thread Stephan Erb 

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



I won't be able to work in this in the near future. Closing.

- Stephan Erb


On Nov. 8, 2017, 11:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63685/
> ---
> 
> (Updated Nov. 8, 2017, 11:32 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Is this something the community would be interested in?
> 
> Known issues in this minimal viable prototype:
> 
> * I did not manage to convince Jetty to perform the redirecting. Using the 
> HTML feels hacky but works.
> * An operator will have difficulties to navigate to /admin of a non-leading 
> instance. In particular if there is no leader at all.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 0f8528c3403b5f51f082aa54c16358f7568f439a 
>   src/main/resources/scheduler/assets/index.html 
> 533091395547a6b067dbf5f53e42a13a560971da 
>   ui/src/main/js/components/Navigation.js 
> 50881a9914cee2812807624cd28f24fa64207296 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/js/pages/Admin.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63685/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-11-08 at 23.18.06.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/80ae2c5f-e431-4f86-8811-13bfe6a8627b__Screen_Shot_2017-11-08_at_23.18.06.png
> Screen Shot 2017-11-08 at 23.18.20.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/9fb0f2e0-49c9-427f-8b84-d3d1fc057a59__Screen_Shot_2017-11-08_at_23.18.20.png
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-09 Thread David McLaughlin 

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




src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
Line 337 (original), 337 (patched)


We shouldn't have to define "UI" routes that are part of the React app in 
the Scheduler. We should explicitly route to /api and the remaining static 
assets left (Thrift stuff, jQuery for Thrift transport layer) and then route 
everything else to /assets/scheduler/index.html and let React Router decide 
what to do.



ui/src/main/js/components/Navigation.js
Lines 15 (patched)


I'm a little wary of putting this in the navigation for all users to see. 
Some of the endpoints are quite expensive and not part of public APIs.



ui/src/main/js/index.js
Lines 38 (patched)


The Admin does not need the API.


- David McLaughlin


On Nov. 8, 2017, 10:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63685/
> ---
> 
> (Updated Nov. 8, 2017, 10:32 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Is this something the community would be interested in?
> 
> Known issues in this minimal viable prototype:
> 
> * I did not manage to convince Jetty to perform the redirecting. Using the 
> HTML feels hacky but works.
> * An operator will have difficulties to navigate to /admin of a non-leading 
> instance. In particular if there is no leader at all.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 0f8528c3403b5f51f082aa54c16358f7568f439a 
>   src/main/resources/scheduler/assets/index.html 
> 533091395547a6b067dbf5f53e42a13a560971da 
>   ui/src/main/js/components/Navigation.js 
> 50881a9914cee2812807624cd28f24fa64207296 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/js/pages/Admin.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63685/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-11-08 at 23.18.06.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/80ae2c5f-e431-4f86-8811-13bfe6a8627b__Screen_Shot_2017-11-08_at_23.18.06.png
> Screen Shot 2017-11-08 at 23.18.20.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/9fb0f2e0-49c9-427f-8b84-d3d1fc057a59__Screen_Shot_2017-11-08_at_23.18.20.png
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-08 Thread Bill Farner 

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



I agree with the direction, and i agree that we should try for a redirect-free 
experience.  Thought added below.


src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java
Line 334 (original), 334 (patched)


I may be misunderstanding the goal, but i would have expected this line to 
change to:
```
.put("/(?:index.html)?", "/assets/scheduler/index.html")
```

This should serve the scheduler UI when accessing `/` or `/index.html`.


- Bill Farner


On Nov. 8, 2017, 2:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63685/
> ---
> 
> (Updated Nov. 8, 2017, 2:32 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Is this something the community would be interested in?
> 
> Known issues in this minimal viable prototype:
> 
> * I did not manage to convince Jetty to perform the redirecting. Using the 
> HTML feels hacky but works.
> * An operator will have difficulties to navigate to /admin of a non-leading 
> instance. In particular if there is no leader at all.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 0f8528c3403b5f51f082aa54c16358f7568f439a 
>   src/main/resources/scheduler/assets/index.html 
> 533091395547a6b067dbf5f53e42a13a560971da 
>   ui/src/main/js/components/Navigation.js 
> 50881a9914cee2812807624cd28f24fa64207296 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/js/pages/Admin.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63685/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-11-08 at 23.18.06.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/80ae2c5f-e431-4f86-8811-13bfe6a8627b__Screen_Shot_2017-11-08_at_23.18.06.png
> Screen Shot 2017-11-08 at 23.18.20.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/9fb0f2e0-49c9-427f-8b84-d3d1fc057a59__Screen_Shot_2017-11-08_at_23.18.20.png
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-08 Thread Joshua Cohen 

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



In general I think the spirit of this change makes sense, but as you mention, 
the HTTP redirect is definitely not ideal. It should certainly be possible to 
serve up the actual scheduler UI by default without the need to redirect. Off 
the top of my head I'm not sure exactly what would need to be done though.

- Joshua Cohen


On Nov. 8, 2017, 10:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63685/
> ---
> 
> (Updated Nov. 8, 2017, 10:32 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Is this something the community would be interested in?
> 
> Known issues in this minimal viable prototype:
> 
> * I did not manage to convince Jetty to perform the redirecting. Using the 
> HTML feels hacky but works.
> * An operator will have difficulties to navigate to /admin of a non-leading 
> instance. In particular if there is no leader at all.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 0f8528c3403b5f51f082aa54c16358f7568f439a 
>   src/main/resources/scheduler/assets/index.html 
> 533091395547a6b067dbf5f53e42a13a560971da 
>   ui/src/main/js/components/Navigation.js 
> 50881a9914cee2812807624cd28f24fa64207296 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/js/pages/Admin.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63685/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-11-08 at 23.18.06.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/80ae2c5f-e431-4f86-8811-13bfe6a8627b__Screen_Shot_2017-11-08_at_23.18.06.png
> Screen Shot 2017-11-08 at 23.18.20.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/9fb0f2e0-49c9-427f-8b84-d3d1fc057a59__Screen_Shot_2017-11-08_at_23.18.20.png
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Re: Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-08 Thread Aurora ReviewBot 

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



Master (5dfe51c) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

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

- Aurora ReviewBot


On Nov. 8, 2017, 10:32 p.m., Stephan Erb wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63685/
> ---
> 
> (Updated Nov. 8, 2017, 10:32 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Is this something the community would be interested in?
> 
> Known issues in this minimal viable prototype:
> 
> * I did not manage to convince Jetty to perform the redirecting. Using the 
> HTML feels hacky but works.
> * An operator will have difficulties to navigate to /admin of a non-leading 
> instance. In particular if there is no leader at all.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 0f8528c3403b5f51f082aa54c16358f7568f439a 
>   src/main/resources/scheduler/assets/index.html 
> 533091395547a6b067dbf5f53e42a13a560971da 
>   ui/src/main/js/components/Navigation.js 
> 50881a9914cee2812807624cd28f24fa64207296 
>   ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
>   ui/src/main/js/pages/Admin.js PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63685/diff/1/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Screen Shot 2017-11-08 at 23.18.06.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/80ae2c5f-e431-4f86-8811-13bfe6a8627b__Screen_Shot_2017-11-08_at_23.18.06.png
> Screen Shot 2017-11-08 at 23.18.20.png
>   
> https://reviews.apache.org/media/uploaded/files/2017/11/08/9fb0f2e0-49c9-427f-8b84-d3d1fc057a59__Screen_Shot_2017-11-08_at_23.18.20.png
> 
> 
> Thanks,
> 
> Stephan Erb
> 
>

Review Request 63685: RFC: Use new scheduler UI as landing page

2017-11-08 Thread Stephan Erb 

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

Review request for Aurora, David McLaughlin and Joshua Cohen.


Repository: aurora


Description
---

Is this something the community would be interested in?

Known issues in this minimal viable prototype:

* I did not manage to convince Jetty to perform the redirecting. Using the HTML 
feels hacky but works.
* An operator will have difficulties to navigate to /admin of a non-leading 
instance. In particular if there is no leader at all.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
0f8528c3403b5f51f082aa54c16358f7568f439a 
  src/main/resources/scheduler/assets/index.html 
533091395547a6b067dbf5f53e42a13a560971da 
  ui/src/main/js/components/Navigation.js 
50881a9914cee2812807624cd28f24fa64207296 
  ui/src/main/js/index.js 9f94d4bd6f649d74bdd9c3aa99783e01cae25d93 
  ui/src/main/js/pages/Admin.js PRE-CREATION 


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


Testing
---


File Attachments


Screen Shot 2017-11-08 at 23.18.06.png
  
https://reviews.apache.org/media/uploaded/files/2017/11/08/80ae2c5f-e431-4f86-8811-13bfe6a8627b__Screen_Shot_2017-11-08_at_23.18.06.png
Screen Shot 2017-11-08 at 23.18.20.png
  
https://reviews.apache.org/media/uploaded/files/2017/11/08/9fb0f2e0-49c9-427f-8b84-d3d1fc057a59__Screen_Shot_2017-11-08_at_23.18.20.png


Thanks,

Stephan Erb