ClusterOverview: refactor Utilization card and connect #307
Conversation
Pull Request Test Coverage Report for Build 1248
💛 - Coveralls |
|
Loading state: |
mareklibra
force-pushed
the
utilization.realData
branch
from
125f995 to
da0e1d0
Compare
|
just wondering, whats the timeout on fetching the data? (in case Prometheus is down for example), do we also show error to load the card? |
|
|
||
| export const getUtilizationCpuStats = response => { | ||
| const values = get(response, 'data.result[0].values', []); | ||
| return values.map(timeValuePair => parseNumber(timeValuePair[1])); |
There was a problem hiding this comment.
I think it would be better to return null if we have no values, instead of empty array - see my other comment about Not available state
| {isLoading || `${actual} ${unit}`} | ||
| </Col> | ||
| <Col className="kubevirt-utilization__item-chart" lg={7} md={7} sm={7} xs={7}> | ||
| {isLoading ? <LoadingComponent /> : <SparklineChart id={id} data={chartData} axis={axis} unloadBeforeLoad />} |
There was a problem hiding this comment.
can you add 'Not available' state ? when isLoading is false and data are null. We already have that in ClusterCapacity https://github.com/kubevirt/web-ui-components/blob/master/src/components/Dashboard/Capacity/CapacityItem.js#L24
| return ( | ||
| <div className="kubevirt-utilization__item"> | ||
| <Row> | ||
| <Col lg={2} md={2} sm={2} xs={2}> |
There was a problem hiding this comment.
looking at https://docs.google.com/presentation/d/1kfzD1jyEAIejWFKoKSTJLCetwvmIriLI2UlFbyB-xbg/edit#slide=id.g54d7130601_45_38 - we need to improve behaviour of UtilizationItem when there is not enough space
There was a problem hiding this comment.
Good catch. Can I deal with that in a separate PR?
There are basically two options
- introduce sort of
isNarrowproperty (preferred) - refactor the component structure and handle it via CSS (too complicated considering the case)
There was a problem hiding this comment.
Sure, lets fix this in followup
mareklibra
force-pushed
the
utilization.realData
branch
from
da0e1d0 to
af7af59
Compare
|
If metric can not be accessed:
|
Utilization card is refactored to split concern between ClusterOverview and Dashboard components. Connected to real data. So far CPU only, others will follow.
mareklibra
force-pushed
the
utilization.realData
branch
from
af7af59 to
8cf91c0
Compare
|
@mareklibra just FYI I have asked Andy to provide us designs for Not Available/ Loading state #310 so once we will have those we may need to change this a bit, however Im happy with the current state |
Utilization card is refactored to split concern between ClusterOverview and Dashboard components. Connected to real data. So far CPU only, others will follow.
Utilization card is refactored to split concern between
ClusterOverview and Dashboard components.
Connected to real data.
So far CPU only, other items will follow.