sodonnel commented on a change in pull request #3078:
URL: https://github.com/apache/ozone/pull/3078#discussion_r805994356
##########
File path:
hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/client/TestReplicationConfig.java
##########
@@ -159,6 +160,35 @@ public void deserializeEC() {
validate(config, EcCodec.valueOf(codec), data, parity, chunkSize);
}
+ @Test
+ public void testECReplicationConfigGetReplication() {
+ assumeECType();
+ HddsProtos.ECReplicationConfig proto =
+ HddsProtos.ECReplicationConfig.newBuilder().setCodec(codec)
+ .setData(data).setParity(parity).setEcChunkSize(chunkSize).build();
+
+ ReplicationConfig config =
+ ReplicationConfig.fromProto(ReplicationType.EC, null, proto);
+
+ String ecReplicationParamsDelimiter = "-";
Review comment:
Do you think it would make sense to make the static
`EC_REPLICATION_PARAMS_DELIMITER` in `ECReplicationConfig` public and then
reference it here? That way, if we ever changed the constant in
`ECReplicationConfig`, it would mean the test doesn't need to be changed.
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/PipelineEndpoint.java
##########
@@ -105,9 +104,9 @@ public Response getPipelines() {
.setDatanodes(datanodes)
.setDuration(duration)
.setStatus(pipeline.getPipelineState())
- .setReplicationFactor(ReplicationConfig
-
.getLegacyFactor(pipeline.getReplicationConfig()).getNumber())
- .setReplicationType(pipeline.getType().toString());
+ .setReplicationFactor(
Review comment:
Would it make more sense to change the parameters here to accept just
"ReplicationConfig" rather than Factor and Type separately? Then internally it
can call the methods on ReplicationConfig it requires?
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/DatanodePipeline.java
##########
@@ -17,22 +17,22 @@
*/
package org.apache.hadoop.ozone.recon.api.types;
-import org.apache.hadoop.hdds.client.ReplicationConfig;
-
import java.util.UUID;
/**
* Metadata object that contains pipeline information of a Datanode.
*/
public class DatanodePipeline {
private UUID pipelineID;
- private ReplicationConfig repConfig;
+ private String replicationType;
+ private String replicationFactor;
private String leaderNode;
- public DatanodePipeline(UUID pipelineID, ReplicationConfig repConfig,
- String leaderNode) {
+ public DatanodePipeline(UUID pipelineID, String replicationType,
Review comment:
Similar comment here - I think it was better to have ReplicationConfig
as the parameter in the constructor and store that in the class, but then in
the accessors we can return the string representations by calling the correct
method on the stored ReplicationConfig?
##########
File path:
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/DatanodePipeline.java
##########
@@ -17,22 +17,22 @@
*/
package org.apache.hadoop.ozone.recon.api.types;
-import org.apache.hadoop.hdds.client.ReplicationConfig;
-
import java.util.UUID;
/**
* Metadata object that contains pipeline information of a Datanode.
*/
public class DatanodePipeline {
private UUID pipelineID;
- private ReplicationConfig repConfig;
+ private String replicationType;
+ private String replicationFactor;
private String leaderNode;
- public DatanodePipeline(UUID pipelineID, ReplicationConfig repConfig,
- String leaderNode) {
+ public DatanodePipeline(UUID pipelineID, String replicationType,
Review comment:
If we cannot store the ReplicationConfig object in the class, perhaps we
could pass ReplicationConfig to the constructor, and inside the constructor
pull out the String values and store them? In general, we are trying to get
away from Factor and Type in the API everywhere, so that would be a good
compromise perhaps?
--
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]
