chihsuan commented on code in PR #10968:
URL: https://github.com/apache/ozone/pull/10968#discussion_r4082543099


##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/types/bucket.types.ts:
##########
@@ -37,6 +37,37 @@ export const BucketLayoutTypeList = [
 export type BucketLayout = typeof BucketLayoutTypeList[number];
 
 
+// Corresponds to the serialized 
org.apache.hadoop.hdds.client.RatisReplicationConfig
+// and StandaloneReplicationConfig. The latter serializes its replicationType 
as
+// STANDALONE, while the enum name used elsewhere is STAND_ALONE, so both 
spellings
+// can reach the UI.
+type BucketRatisReplicationConfig = {

Review Comment:
   > One knock-on effect worth flagging. openKeysTable already consumes 
ReplicationInfo, and its Replication Factor column narrows on RATIS and treats 
everything else as EC:
   
   Thanks for flagging the `openKeysTable` knock-on. I noticed the v1 OM page 
has the same ternary, so tsc now complains there too: 
https://github.com/apache/ozone/blob/a9544892304a8bb1734092ccf2d0899b50e33ff5/hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/views/insights/om/om.tsx#L220-L222.
   
   Would it make sense to flip that one as well? Fine if it's a follow-up.



##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/types/insights.types.ts:
##########
@@ -119,7 +119,16 @@ export interface EcInfo {
   minimumNodes: number;
 }
 
-export type ReplicationInfo = RatisInfo | EcInfo;
+// StandaloneReplicationConfig serializes its replicationType as STANDALONE,
+// without the underscore used by the ReplicationType enum name STAND_ALONE
+export interface StandaloneInfo {
+  replicationType: 'STANDALONE' | 'STAND_ALONE';

Review Comment:
   Could we drop `STAND_ALONE` here? The nested replication type is always 
`STANDALONE` on a real cluster, so we can drop the extra label too.



##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/__tests__/buckets/BucketsTable.test.tsx:
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { vi } from 'vitest';
+import { render, screen } from '@testing-library/react';
+
+import BucketsTable from '@/v2/components/tables/bucketsTable';

Review Comment:
   nit: Could we add the `React` import like the other test files? tsc flags 
the JSX without it.



##########
hadoop-ozone/recon/src/main/resources/webapps/recon/ozone-recon-web/src/v2/types/bucket.types.ts:
##########
@@ -37,6 +38,14 @@ export const BucketLayoutTypeList = [
 export type BucketLayout = typeof BucketLayoutTypeList[number];
 
 
+// Corresponds to the serialized 
org.apache.hadoop.hdds.client.DefaultReplicationConfig
+// returned by the Recon bucket endpoint 
(BucketObjectDBInfo#replicationConfigInfo).
+// The nested config is the same shape the OM DB insights endpoints return.
+export type BucketReplicationConfig = {
+  type: string;
+  replicationConfig?: ReplicationInfo | null;

Review Comment:
   Could we drop `STAND_ALONE` here? The nested replication type is always 
`STANDALONE` on a real cluster.
   
   
https://github.com/apache/ozone/blob/a9544892304a8bb1734092ccf2d0899b50e33ff5/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/StandaloneReplicationConfig.java#L92-L101



-- 
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]

Reply via email to