UI: Use stats trackers for resource rows #4726
Conversation
DingoEatingFuzz
commented
Sep 26, 2018
DingoEatingFuzz
commented
- Update the stat polling on alloc rows to use the new stat tracker
- Add resource tracking to task rows on the alloc detail page
- Remove dead code from the old stat polling method.
There was a problem hiding this comment.
I could see the stats tracker bars but they didn't seem to be showing any results? Plus the tooltip was showing '/ 2000' (missing the left side of the /). I think I'm checking the right place? Should I be seeing these filled with color/with numbers?
I think apart from that I also noticed that when you are hovered over the stats tracker, and you can see the tooltip, the tooltip seems to 'jump' every 2 seconds or so? The DOM seems to be changing every couple of seconds even though there are no changes to the displayed data?
Anyway, hopefully I'm looking in the right place, let me know and I can try again if you are seeing different to me.
| {{x-icon "warning" class="is-warning"}} | ||
| </span> | ||
| {{else}} | ||
| <div class="inline-chart is-small tooltip" aria-label="{{cpu.used}} / {{taskStats.reservedCPU}} MHz"> |
There was a problem hiding this comment.
There's a role="tooltip" that might come in here?
|
I suspect that the visual issues you are seeing here and in the other PR are related to fixture data. Which I should fix, but in the meantime if you proxy to the demo site, I think things will work fine for you. |
|
@DingoEatingFuzz ah cool, will see if I can give it a shot with that, will see if I can catch up with you later also. |
|
Quick note, I pointed it to the demo site and set |
|
Hey, these are looking cool! The 'jumping'' thing I mentioned above has gone also, so that must have been the fixtures. Semi-related thing I noticed is that when you click away from the Allocation page (the one that has the graphs and the 'Task' table) using the second to last breadcrumb, you hit the following:
It's pretty consistent for me, I've tried in a few other places and it doesn't error (but I did find another one which seems unrelated), it seems to only be clicking the second to last breadcrumb. Cheers, John |
|
I was able to reproduce the bug you observed and tracked it down to a bad namespace lookup for that particular route. I'd prefer to fix that bug in a different PR though since this work isn't what introduced the issue. |
| // Internal state | ||
| statsError: false, | ||
|
|
||
| enablePolling: computed(() => !Ember.testing), |
There was a problem hiding this comment.
Couple of question on this for my own benefit really:
- Could you use
aliasfor this? - Does
Ember.testingever change once things are running?
There was a problem hiding this comment.
Alias only works for paths on the object already (e.g., this.Ember.testing), so no.
I just tried it and yes, Ember.testing can be changed at runtime, but it never happens from framework code.
Cool yeah, sounds good, ping me when you do it and I can take a look again. I gave it another code once over and a couple of q's arose from that just for my own benefit. Other than that LGTM! |
|
Thanks for the review @johncowen, I added |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
