You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/05/27 05:58:58 UTC

[GitHub] [ozone] elek opened a new pull request #2287: HDDS-5168. Use ReplicationConfig in OmKeyArgs

elek opened a new pull request #2287:
URL: https://github.com/apache/ozone/pull/2287


   ## What changes were proposed in this pull request?
   
   During the implementation of HDDS-5145 I realized that OmKeyArgs also uses factor/type, it seems to be easier to convert it to replicationConfig as it's an in-memory class not a protobuf which is required to be persisted.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5168
   
   ## How was this patch tested?
   
   Full CI test.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] umamaheswararao merged pull request #2287: HDDS-5168. Use ReplicationConfig in OmKeyArgs

Posted by GitBox <gi...@apache.org>.
umamaheswararao merged pull request #2287:
URL: https://github.com/apache/ozone/pull/2287


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] sodonnel commented on a change in pull request #2287: HDDS-5168. Use ReplicationConfig in OmKeyArgs

Posted by GitBox <gi...@apache.org>.
sodonnel commented on a change in pull request #2287:
URL: https://github.com/apache/ozone/pull/2287#discussion_r640546237



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneMultipartUploadPartListParts.java
##########
@@ -77,8 +79,16 @@ public boolean isTruncated() {
     return partInfoList;
   }
 
+  @Deprecated
+  public ReplicationType getReplicationType() {
+    return ReplicationType
+            .fromProto(replicationConfig.getReplicationType());
+  }
+
+  @Deprecated
   public ReplicationFactor getReplicationFactor() {
-    return replicationFactor;
+    return ReplicationFactor
+            .fromProto(ReplicationConfig.getLegacyFactor(replicationConfig));
   }

Review comment:
       Maybe we should add a `getReplicationConfig()` here, as we are deprecating the getType and Factor methods?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] umamaheswararao commented on pull request #2287: HDDS-5168. Use ReplicationConfig in OmKeyArgs

Posted by GitBox <gi...@apache.org>.
umamaheswararao commented on pull request #2287:
URL: https://github.com/apache/ozone/pull/2287#issuecomment-852434978


   Changes looks good to me. I see. I think currently deprecated APIs used in the place where we may want to use getReplicationConfig API. I think thats ok for now? they are existing calls already( to deprecated APIs) and eventually new usage required to add that API(getReplicationConfig) and use it. If we wanted to add now, thats ok too. Thanks 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on a change in pull request #2287: HDDS-5168. Use ReplicationConfig in OmKeyArgs

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #2287:
URL: https://github.com/apache/ozone/pull/2287#discussion_r643931964



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneMultipartUploadPartListParts.java
##########
@@ -77,8 +79,16 @@ public boolean isTruncated() {
     return partInfoList;
   }
 
+  @Deprecated
+  public ReplicationType getReplicationType() {
+    return ReplicationType
+            .fromProto(replicationConfig.getReplicationType());
+  }
+
+  @Deprecated
   public ReplicationFactor getReplicationFactor() {
-    return replicationFactor;
+    return ReplicationFactor
+            .fromProto(ReplicationConfig.getLegacyFactor(replicationConfig));
   }

Review comment:
       Sure, I added the getter. It's not yet used, but can be useful for users of the Java API.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] elek commented on pull request #2287: HDDS-5168. Use ReplicationConfig in OmKeyArgs

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2287:
URL: https://github.com/apache/ozone/pull/2287#issuecomment-853002727


   Thanks the review @sodonnel and @umamaheswararao 
   
   >  I think currently deprecated APIs used in the place where we may want to use getReplicationConfig API.
   
   @umamaheswararao Can you please add some examples: I see examples where `S3StorageType` is used (which should be extended anyway), but not sure what other usages are in your mind...


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org