You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by janisz <gi...@git.apache.org> on 2017/09/13 19:13:46 UTC
[GitHub] mesos pull request #233: Display task health in the Web UI.
GitHub user janisz opened a pull request:
https://github.com/apache/mesos/pull/233
Display task health in the Web UI.
Fixes: MESOS-7961
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/janisz/mesos MESOS-7961/disaply_task_health_state_in_web_UI
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/mesos/pull/233.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #233
----
commit c9faf8ced314860a77a8f2742ee1db4677d0075a
Author: Tomasz Janiszewski <to...@allegrogroup.com>
Date: 2017-09-13T19:10:39Z
Display task health in the Web UI.
Fixes: MESOS-7961
----
---
[GitHub] mesos pull request #233: Display task health in the Web UI.
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/mesos/pull/233
---
[GitHub] mesos pull request #233: Display task health in the Web UI.
Posted by bmahler <gi...@git.apache.org>.
Github user bmahler commented on a diff in the pull request:
https://github.com/apache/mesos/pull/233#discussion_r142247344
--- Diff: src/webui/master/static/js/controllers.js ---
@@ -838,6 +839,15 @@
});
}
+ function setHealth(tasks) {
+ _.each(tasks, function(task) {
+ var lastStatus = _.last(task.statuses);
+ if (lastStatus) {
+ task.healthy = lastStatus.healthy;
--- End diff --
2 space indents
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by janisz <gi...@git.apache.org>.
Github user janisz commented on the issue:
https://github.com/apache/mesos/pull/233
@bmahler Thanks for the review. Most of the issues are minor and easy to fix. I do it shortly.
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by janisz <gi...@git.apache.org>.
Github user janisz commented on the issue:
https://github.com/apache/mesos/pull/233
@Vish0007 To create this screenshot I created 3 applications called `unhealthy`, `healthy` and `unknown` so it's just a coincidence that the name has the same value as tasks health. When I fix all issues I'll prepare gif to present healthy is available in all places and I'll use more generic names in order to avoid confusion.
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by bmahler <gi...@git.apache.org>.
Github user bmahler commented on the issue:
https://github.com/apache/mesos/pull/233
Thanks janisz, this looks pretty good. However, this needs to be added to several places:
- The global tasks table (you covered this)
- The framework-specific task table (when clicking into a framework)
- The executor-specific task table within an agent (when clicking into an agent, then a framework, then an executor)
I'm also a bit curious about whether it's a good idea to re-use the health string as a class, since it seems easy to break vs only specifying the display string in the html page.
Will you be able to continue working on this?
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by lilysamimi <gi...@git.apache.org>.
Github user lilysamimi commented on the issue:
https://github.com/apache/mesos/pull/233
Would be interested in talking with the folks that created this UI component. I'm giving out $10 Amazon gift cards to individuals who participate in a user experience research study with me pertaining to the Mesos UI. Please DM me. I'm at MesosCon today and tomorrow and around the Mesosphere booth.
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by Vish0007 <gi...@git.apache.org>.
Github user Vish0007 commented on the issue:
https://github.com/apache/mesos/pull/233
@janisz the UI looks good and health check does add value.
would you describe how the "Task Name" and "Health" are different, i can see health status in both places.
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by Vish0007 <gi...@git.apache.org>.
Github user Vish0007 commented on the issue:
https://github.com/apache/mesos/pull/233
@janisz sounds great.
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by janisz <gi...@git.apache.org>.
Github user janisz commented on the issue:
https://github.com/apache/mesos/pull/233
* [x] The global tasks table
* [x] The framework-specific task table (when clicking into a framework)
* [x] The executor-specific task table within an agent (when clicking into an agent, then a framework, then an executor)
> I'm also a bit curious about whether it's a good idea to re-use the health string as a class, since it seems easy to break vs only specifying the display string in the html page.
I removed CSS. I'm not sure if red/green looks good in Mesos blue palette. @lilysamimi Any idea how to present health state?
<details>
I used Marathon to deploy 3 applications with 3 types of health state.
```
curl -X PUT -d@health_groups.json localhost:8080/v2/groups/
```
```json
{
"apps":[
{
"id":"/apache",
"cmd":"sleep 1000",
"cpus":1,
"mem":128,
"disk":0,
"instances":1,
"healthChecks":[
{
"gracePeriodSeconds":1,
"intervalSeconds":1,
"maxConsecutiveFailures":300,
"command":{
"value":"true"
},
"protocol":"COMMAND"
}
]
},
{
"id":"/tomcat",
"cmd":"sleep 1000",
"cpus":1,
"mem":128,
"disk":0,
"instances":1,
"healthChecks":[
{
"gracePeriodSeconds":1,
"intervalSeconds":1,
"maxConsecutiveFailures":300,
"command":{
"value":"false"
},
"protocol":"COMMAND"
}
]
},
{
"id":"/mysql",
"cmd":"sleep 1000",
"cpus":1,
"mem":128,
"disk":0,
"instances":1
}
]
}
```
</details>
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by janisz <gi...@git.apache.org>.
Github user janisz commented on the issue:
https://github.com/apache/mesos/pull/233
@bmahler I added some colours.
---
[GitHub] mesos pull request #233: Display task health in the Web UI.
Posted by janisz <gi...@git.apache.org>.
Github user janisz commented on a diff in the pull request:
https://github.com/apache/mesos/pull/233#discussion_r140917021
--- Diff: src/webui/master/static/home.html ---
@@ -162,14 +162,15 @@
<th data-key="name">Task Name</th>
--- End diff --
Task ID is one line above
---
[GitHub] mesos pull request #233: Display task health in the Web UI.
Posted by Vishant0031 <gi...@git.apache.org>.
Github user Vishant0031 commented on a diff in the pull request:
https://github.com/apache/mesos/pull/233#discussion_r140641268
--- Diff: src/webui/master/static/home.html ---
@@ -162,14 +162,15 @@
<th data-key="name">Task Name</th>
--- End diff --
is this task id (MESOS_TASK_ID?), should this be "Task Id"?
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by bmahler <gi...@git.apache.org>.
Github user bmahler commented on the issue:
https://github.com/apache/mesos/pull/233
@janisz I noticed you updated the patch, is it ready for another review? Would be great to keep the colors as you had before. I was just asking about the approach used :)
Also can you add a gif or video of your screen to show the testing of it?
---
[GitHub] mesos issue #233: Display task health in the Web UI.
Posted by janisz <gi...@git.apache.org>.
Github user janisz commented on the issue:
https://github.com/apache/mesos/pull/233
@kwiatkk1 Thanks for the review. I applied your suggestions.
@bmahler Changed `Unknown` to `-`
---
[GitHub] mesos pull request #233: Display task health in the Web UI.
Posted by bmahler <gi...@git.apache.org>.
Github user bmahler commented on a diff in the pull request:
https://github.com/apache/mesos/pull/233#discussion_r142247472
--- Diff: src/webui/master/static/js/app.js ---
@@ -88,6 +88,14 @@
return state.substring(5);
};
})
+ .filter('taskHealth', function() {
+ return function(health) {
+ if (health != null) {
+ return health ? "healthy" : "unhealthy";
--- End diff --
2 space indent, would also be good to warn the reader here that this value is relied on for the CSS
---