-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI: Driver health checking #4294
Conversation
f897c6b
to
7f4adc2
Compare
classNames: ['accordion'], | ||
|
||
key: 'id', | ||
source: computed(() => []), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah not seen this, I've been doing it in init
I think, nice!
if (!Ember.testing) { | ||
this.set('_visibilityHandler', this.get('visibilityHandler').bind(this)); | ||
document.addEventListener('visibilitychange', this.get('_visibilityHandler')); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this similar to that on click outside not cleaning up thing I had during testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had to do with requests canceling and resulting in runloop errors being thrown in tests when I was multitasking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 👍 I really like the list-accordion
component design.
head=(component "list-accordion/accordion-head" | ||
isOpen=item.isOpen | ||
onOpen=(action "open" item) | ||
onClose=(action "close" item)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could use (action (mut item.isOpen) true)
and (action (mut item.isOpen) false)
here instead, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I always forget about mut
.
.reduce((all, drivers) => { | ||
all.push(...drivers); | ||
return all; | ||
}, []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If only we had Array.prototype.smoosh
…
ui/app/serializers/node.js
Outdated
if (this.get('config.isDev') && hash.HTTPAddr === '127.0.0.1:4646') { | ||
hash.HTTPAddr = '127.0.0.1:4200'; | ||
} | ||
// Transform the map-based Drivers object into an array-based ClientDriver fragment list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment refers to ClientDriver
, but I think the fragment model name is NodeDriver
. Am I mistaken?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, yeah. We use Node and Client sorta but not really interchangeably. I try to keep all the data layers stuff using Node and all the routes and copy saying Client. Really easy to get wires crossed.
This was used to get around direct requests to clients. The UI will now automatically route through the server.
Follows the same style as the table and pagination components.
The warning shows up when the task's driver is unhealthy on the node the task is running on.
It results in surprise behaviors.
7f4adc2
to
c875d14
Compare
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. |
This adds a few things:
Allocations on the task group and client pages are denoted when they depend on an unhealthy driver
The client detail page now shows all drivers and their status, (i.e., detected and healthy)
Drivers can be expanded to show details including driver-specific attributes and a message regarding health
Client events include the subsystem the event is for and a message