chihsuan commented on code in PR #10577:
URL: https://github.com/apache/ozone/pull/10577#discussion_r3562937161
##########
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:
This looks like it fixes a real null-deref. Could you mention it in the PR
description? Feels worth surfacing since it's a behaviour fix, not just a
test tweak.
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/components/cards/overviewHealthCard.tsx:
##########
@@ -56,7 +49,7 @@ const cardBodyStyle: React.CSSProperties = {
// ------------- Component -------------- //
-const OverviewSummaryCard: React.FC<OverviewTableCardProps> = ({
+const OverviewHealthCard: React.FC<OverviewTableCardProps> = ({
Review Comment:
Same thought as the `reserved` field. The rename seems unrelated to the
dependency bump. 🤔
##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/pages/overview/overview.tsx:
##########
@@ -232,10 +232,10 @@ const Overview: React.FC<{}> = () => {
<WrappedInfoIcon title={totalCapacityDesc} />
</span>
),
- value: storageReport.capacity,
+ value: storageReport?.capacity ?? 'N/A',
Review Comment:
nit: `CapacityBreakdown` types value as number and renders `0 B` for
anything `<= 0`, so this `'N/A'` never actually shows. It always displays `0 B.
?? 0` would match the sibling `OTHER USED SPACE` and the prop type. The same
applies to capacity and committed just below.
https://github.com/apache/ozone/blob/423b3dabe686312536f2fbcc3915d40cc80d5506/hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/pages/capacity/components/CapacityBreakdown.tsx#L60
##########
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:
This one I couldn't quite tie back to the `msw/vitest` upgrade. Is the new
`reserved` field needed to get the tests passing here, or is it independent?
--
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]
