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


Fix it, then Ship it!




Thanks for doing this!!!

js newbee here. Just some minor comments. The screenshot looks great to me!


src/webui/app/app.js
Lines 276 (patched)
<https://reviews.apache.org/r/71178/#comment305099>

    s/g/'g'/



src/webui/app/frameworks/roles.html
Lines 1 (patched)
<https://reviews.apache.org/r/71178/#comment305097>

    Dose this mean, if the framework has less than three roles, we will just 
join them with space?
    
    I would still prefer a list even if there are only two roles, will be 
clearer and consistent?


- Meng Zhu


On Aug. 23, 2019, 10:34 a.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71178/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2019, 10:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-8503
>     https://issues.apache.org/jira/browse/MESOS-8503
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch makes the UI pages `frameworks` and `framework` display
> roles of multi-role frameworks as collapsible tree (instead of a list).
> 
> 
> Diffs
> -----
> 
>   src/webui/app/app.js f6f11384f35b260dcaff432c884dc063ea5e0f0e 
>   src/webui/app/frameworks/framework.html 
> 82f6b279a9416d147fcfab094a326d67d0951dcc 
>   src/webui/app/frameworks/frameworks.html 
> d37c6137b638a27e5bd0f70f08733d81550b3ace 
>   src/webui/app/frameworks/roles-tree-root.html PRE-CREATION 
>   src/webui/app/frameworks/roles-tree.html PRE-CREATION 
>   src/webui/app/frameworks/roles.html PRE-CREATION 
>   src/webui/assets/css/mesos.css 0ff47cda36cc897f2e8f43804d38046f3e27e575 
> 
> 
> Diff: https://reviews.apache.org/r/71178/diff/3/
> 
> 
> Testing
> -------
> 
> Tested manually with conbination of frameworks with thousands of roles and 
> frameworks with 1-2 roles.
> 
> Performance: re-rendering time of `frameworks` page with 50 frameworks with 
> ~4000 roles each is around 500 ms on my hardware. 
> With all subtrees collapsed, more than half of this time is spent inside 
> `intermediateRoleTree()` and `aggregateRoleTree()` (roughly equal amounts of 
> time).
> If/when the master starts to store the roles as a tree, it might make sense 
> to convert this to getting roles in the tree form.
> 
> 
> File Attachments
> ----------------
> 
> Frameworks table: collapsed tree, expanded tree and a framework with only two 
> rolesnd a framework e
>   
> https://reviews.apache.org/media/uploaded/files/2019/08/23/ce65874e-56f2-4ddd-9135-f428515a193b__brief_roles.png
> "Framework" page with a roles tree
>   
> https://reviews.apache.org/media/uploaded/files/2019/08/23/f99ddea3-afb6-44d2-9e1b-fb1f9fdb77e8__framework_page.png
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Reply via email to