NAS-131317 / 25.04 / Replaced Lifetime with Power On Hours #10845
Conversation
aiden3c
requested review from
bvasilenko and
a team
and removed request for
a team
bugclerk
changed the title
|
Oh another thing I added was a "toolTip" property to our ColumnComponent, and the ix table header will use that in place of the default tooltip (just reiterating the title) if it exists. There's some public facing verbiage so it's worth reviewing that too. Just the explanation of what POH are |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #10845 +/- ##
=======================================
Coverage 81.79% 81.80%
=======================================
Files 1588 1588
Lines 55082 55083 +1
Branches 5805 5805
=======================================
+ Hits 45055 45059 +4
+ Misses 10027 10024 -3 ☔ View full report in Codecov by Sentry. |
| @@ -5,6 +5,7 @@ import { DataProvider } from 'app/modules/ix-table/interfaces/data-provider.inte | |||
| export abstract class ColumnComponent<T> { | |||
| propertyName?: keyof T; | |||
| title?: string; | |||
| toolTip?: string; | |||
There was a problem hiding this comment.
Better call it headerTooltip.
| @@ -84,6 +89,7 @@ describe('SmartTestResultListComponent', () => { | |||
| lba_of_first_error: null, | |||
| status: SmartTestResultStatus.Success, | |||
| remaining: 0, | |||
| poh_ago: 0, | |||
There was a problem hiding this comment.
Maybe add at least one value, so that it appears in the table below.
| propertyName: 'lifetime', | ||
| title: this.translate.instant('Power On Hours Ago'), | ||
| propertyName: 'poh_ago', | ||
| toolTip: '"Power On Hours" are how many hours have passed while the disk has been powered on. "Power On Hours Ago" is how many power on hours have passed since each test.', |
There was a problem hiding this comment.
Please wrap string with this.translate.instant so that it gets extracted into translation messages.
| @@ -52,6 +52,7 @@ export interface SmartTestResult { | |||
| status: SmartTestResultStatus; | |||
| status_verbose: string; | |||
| segment_number: number; | |||
| poh_ago: number; | |||
There was a problem hiding this comment.
poh_ago, such a descriptive name 😅
There was a problem hiding this comment.
Looks good, tested via mocks. As long as middleware has power_on_hours_ago in the response, it'll be shown correctly.
Let's make the following adjustments:
- Add
(click)="$event.stopPropagation()"toix-tooltipinix-table-head, so that it doesn't trigger column sorting when tooltip is opened. - Align it vertically to be inline with the text.
- Test fails in part because code picks up test in the tooltip. You can replace
Power On Hours Agoin expect block withexpect.stringContaining('Power On Hours Ago')for now.
|
This PR has been merged and conversations have been locked. |
I replaced the "Lifetime" value in our smart test results with "Power On Hours Ago". This reports how many "power on hours" ago a test happened.
Power on hours are how many hours a disk has been on, the "hours ago" is the disks' total POH - the disks' POH when the test was initiated.
It would be really nice if we could track the actual timestamp of test starts, but smartctl doesn't supply that information. We could track it ourselves, but that becomes very complicated after tests overwrite their own history (scsi only reports the most recent 20 for example). I've thought of some ways to maybe track the actual "hours ago" a test was performed, but it always balloons out. After discussion with Caleb we decided that POH would be the best bet.
Regardless, it's more useful and readable than the raw disk lifetime value.
This has to be paired with this middleware PR, otherwise the key won't exist when getting the results.