You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Russole (via GitHub)" <gi...@apache.org> on 2026/02/19 04:26:25 UTC

[PR] HDDS-14655. Add a generic type to ScmCodec [ozone]

Russole opened a new pull request, #9789:
URL: https://github.com/apache/ozone/pull/9789

   ## What changes were proposed in this pull request?
   * Add generics to ScmCodec and its implementations.
   * Remove redundant InvalidProtocolBufferException from simple codecs.
   * Remove unnecessary type casts in tests after generic refactor.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-14655
   ## How was this patch tested?
   - All CI checks passed.
      -  https://github.com/Russole/ozone/actions/runs/22166418061
   
   


-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-14655. Add a generic type to ScmCodec [ozone]

Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on PR #9789:
URL: https://github.com/apache/ozone/pull/9789#issuecomment-3925717233

   I see. Thanks @szetszwo for this info. 
   
   


-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-14655. Add a generic type to ScmCodec [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9789:
URL: https://github.com/apache/ozone/pull/9789#issuecomment-3926446090

   Thanks @Russole for the patch, @Gargi-jais11, @szetszwo for the review.


-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-14655. Add a generic type to ScmCodec [ozone]

Posted by "Gargi-jais11 (via GitHub)" <gi...@apache.org>.
Gargi-jais11 commented on code in PR #9789:
URL: https://github.com/apache/ozone/pull/9789#discussion_r2825748007


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmGeneratedMessageCodec.java:
##########
@@ -26,7 +26,7 @@
 /**
  * {@link ScmCodec} for {@link Message} objects.
  */
-public class ScmGeneratedMessageCodec implements ScmCodec {
+public class ScmGeneratedMessageCodec implements ScmCodec<Object> {

Review Comment:
   @Russole Why we are using Object here? I think it's better to use `ScmCodec<Message>` because the PR is intedented that subclass implementations does not have to cast the objects in serialize or deserialize. 



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmEnumCodec.java:
##########
@@ -28,7 +28,7 @@
 /**
  * {@link ScmCodec} for {@link ProtocolMessageEnum} objects.
  */
-public class ScmEnumCodec implements ScmCodec {
+public class ScmEnumCodec implements ScmCodec<Object>  {

Review Comment:
   Why not ScmCodec<ProtocolMessageEnum> ?so need not to cast during serialize



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmNonShadedGeneratedMessageCodec.java:
##########
@@ -28,7 +28,7 @@
  * {@link ScmCodec} implementation for non-shaded
  * {@link com.google.protobuf.Message} objects.
  */
-public class ScmNonShadedGeneratedMessageCodec implements ScmCodec {
+public class ScmNonShadedGeneratedMessageCodec implements ScmCodec<Object> {

Review Comment:
   Similar question over here as well, why not to use ScmCodec<Message>?



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-14655. Add a generic type to ScmCodec [ozone]

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #9789:
URL: https://github.com/apache/ozone/pull/9789#discussion_r2826327228


##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/io/ScmEnumCodec.java:
##########
@@ -28,7 +28,7 @@
 /**
  * {@link ScmCodec} for {@link ProtocolMessageEnum} objects.
  */
-public class ScmEnumCodec implements ScmCodec {
+public class ScmEnumCodec implements ScmCodec<Object>  {

Review Comment:
   @Gargi-jais11 , thanks for reviewing this!
   
   This and other are the complicated cases described in [this comment](https://issues.apache.org/jira/browse/HDDS-14655?focusedCommentId=18059431&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-18059431).  We will fix them in separated JIRAs.



-- 
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: issues-unsubscribe@ozone.apache.org

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


Re: [PR] HDDS-14655. Add a generic type to ScmCodec [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #9789:
URL: https://github.com/apache/ozone/pull/9789


-- 
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: issues-unsubscribe@ozone.apache.org

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