Web-console: update supervisors table#7799
Conversation
clintropolis
left a comment
clintropolis
left a comment
There was a problem hiding this comment.
Neat 👍
Note that this PR should not be merged until #7428 is merged.
| } | ||
| } | ||
|
|
||
| function detailedStatusToColor(status: string): string { |
There was a problem hiding this comment.
I think coloring should be probably tied to status instead of detailedStatus, as detailedStatus is messaging specific to the type of supervisor, and 'status' is intended to be universal. Values are:
UNHEALTHY_SUPERVISOR
UNHEALTHY_TASKS
PENDING
RUNNING
SUSPENDED
STOPPING
To me it probably makes sense to use 'status' to set the color, but maybe display the detailed messaging?
|
Marking as WIP - current thinking is for performance reasons it may make sense to implement this on top of #7007, which doesn't exist yet. |
|
As a more near term fix while we wait for #7007 to be in place, I've opened #7839 which at least allows fetching the list of supervisors in a way that includes the 'state', 'detailedState', and 'health' information added by #7428, without the full overhead of getting a supervisor status report. The information visible will have to be cut down from the current form of this PR (no partition count, replica count, lag metric), but the important/high utility bit of displaying the supervisor status and health will be available. |
|
@mcbrewster could you please re-orient this PR to be around #7839 and not to do an API call per supervisor to populate the supervisors table? |
| import './tasks-view.scss'; | ||
|
|
||
| const supervisorTableColumns: string[] = ['Datasource', 'Type', 'Topic/Stream', 'Status', ActionCell.COLUMN_LABEL]; | ||
| const supervisorTableColumns: string[] = ['Datasource', 'Type', 'Topic/Stream', 'Detailed status', ActionCell.COLUMN_LABEL]; |
There was a problem hiding this comment.
I don't see a reason to change the column name, in the API the new field is detailedStatus because status is already there but as far as the user is concerned it is the same thing (just more detailed)
| processQuery: async (query: string) => { | ||
| const resp = await axios.get('/druid/indexer/v1/supervisor?full'); | ||
| const data = resp.data; | ||
| data.forEach( async (item: any ) => { |
There was a problem hiding this comment.
I though the whole point was not to have a query per row of the table, use only the info from /druid/indexer/v1/supervisor?full
| if (!data) return ''; | ||
| const { payload } = data; | ||
| if (!payload) return ''; | ||
| const value = payload.detailedState; |
| return <span> | ||
| <span | ||
| style={{ color: value === 'Suspended' ? '#d58512' : '#2167d5' }} | ||
| style={{ color: payload.healthy ? '#2167d5' : '#d5100a'}} |
There was a problem hiding this comment.
surely there are more statuses (and thus more colors) now. How does healthy relate to detailed status?
| const value = row.value; | ||
| width: 300, | ||
| accessor: (row) => { | ||
| console.log(row); |
There was a problem hiding this comment.
console.log
Also
I think accessor should not return a react element but a string
Save the rendering for the Cell
|
This looks great 👍 |
* add new columns * fix syling * fix spaces * update snapshots * fix Spelling * fix capitalization * reorder action dialog * set color using state * fix snapshots * fix array * update snapshots * remove extra columns * update snapshots * update snapshots * fixes * update snapshots * use cell * fix spacing * update snapshot
* add new columns * fix syling * fix spaces * update snapshots * fix Spelling * fix capitalization * reorder action dialog * set color using state * fix snapshots * fix array * update snapshots * remove extra columns * update snapshots * update snapshots * fixes * update snapshots * use cell * fix spacing * update snapshot
4 participants
Changes status to show detailed status and the color indicator now reflects the health status of the supervisor.