chihsuan commented on code in PR #10577:
URL: https://github.com/apache/ozone/pull/10577#discussion_r3623014668
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/package.json:
##########
@@ -77,13 +77,13 @@
"globals": "^16.5.0",
"jsdom": "^29.1.1",
"json-server": "^0.17.4",
- "msw": "1.3.3",
+ "msw": "^2.15.0",
"npm-run-all2": "^9.0.2",
"prettier": "^2.8.4",
"typescript-eslint": "^8.63.0",
"vite": "6.4.3",
"vite-tsconfig-paths": "^3.6.0",
- "vitest": "^1.6.1"
+ "vitest": "^3.2.7"
Review Comment:
The PR says `MSW 2.14.6 / Vitest 3.2.6`, while this uses `2.15.0 / 3.2.7`.
These were changed in
https://github.com/apache/ozone/pull/10577/changes/0a25c7371e7a3239ee8001c817acff0c2cf2d225
Is this intentional?
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/components/storageBar/storageBar.tsx:
##########
@@ -80,7 +80,7 @@ const StorageBar: React.FC<StorageReportProps> = ({
</tr>
<tr>
<td>Filesystem Reserved</td>
- <td><strong>{size(fsReserved)}</strong></td>
+ <td><strong>{size(fsReserved as number)}</strong></td>
Review Comment:
Could we avoid casting `fsReserved` here? reserved is optional, while
`hasFilesystemView` only verifies capacity and available space. If it is
missing, `filesize(undefined)` throws during render. We could conditionally
render this row, or make reserved required in `DatanodeStorageReport` if the
API guarantees it.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/pages/overview/overview.tsx:
##########
@@ -172,7 +172,7 @@ const Overview: React.FC<{}> = () => {
openContainers,
omServiceId,
scmServiceId
- } = clusterState.data;
+ } = clusterState.data ?? {};
Review Comment:
After looking into this more, I wonder if we should treat a 200 response
with a `null` body as an invalid response instead of falling back to `{}`?
Right now it renders zero values and can even show Datanodes as Healthy
(N/A/N/A). `DEFAULT_CLUSTER_STATE` would be a cleaner fallback, but we’d still
need an error/unknown state.
https://github.com/apache/ozone/blob/256ac0e8cefc40ce084749521598b11a76cbd87d/hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/components/cards/overviewHealthCard.tsx#L86
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/types/datanode.types.tsx:
##########
@@ -29,6 +29,7 @@ export type DatanodeOpState = DatanodeOpStateType[number];
export interface IStorageReport {
capacity: number;
used: number;
+ reserved: number;
Review Comment:
Correct me if I'm wrong, I think this type is used by the older v1 views,
which neither read reserved nor provide it in all their object literals. The
v2 storage UI uses the separate `DatanodeStorageReport `type, where reserved is
already declared, so we shouldn't need to add this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
