-
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: CSI Availability Gauges #7911
Conversation
ui/app/components/gauge-chart.js
Outdated
this.renderChart(); | ||
}, | ||
|
||
mountD3Elements() {}, |
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 intentionally empty ?
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.
Lol, whoops. You caught me copy/pasting. Will delete 👍
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 is beautiful. Mostly LGTM, but I have one question about the empty mountD3Elements
method
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.
I mostly ignored the changes that I think come from #7896 but this looks good to me, I did also wonder about that empty D3 function.
ui/app/components/gutter-menu.js
Outdated
// a storage-related page, context should be reset to volumes. | ||
const destination = this.router.currentRouteName.startsWith('csi.') ? 'csi.volumes' : 'jobs'; | ||
|
||
this.router.transitionTo(destination, { |
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 was already merged, right?
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.
Yes. I must have missed a spot when doing git surgery.
label: 'Gauge', | ||
}); | ||
|
||
test('presents as an svg, a formatted percentage, and an svg', async function(assert) { |
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 meant to be “and a label” maybe?
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.
fixed.
f654516
to
0a258b1
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. |
They look like this and are used to measure how many instances of CSI node and controller plugins are in a healthy state:
Includes: