You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2022/03/03 14:00:27 UTC

[GitHub] [ratis] captainzmc opened a new pull request #617: RATIS-1547. Make GrpcClientProtocolService and log to public

captainzmc opened a new pull request #617:
URL: https://github.com/apache/ratis/pull/617


   ## What changes were proposed in this pull request?
   
   see: https://issues.apache.org/jira/browse/RATIS-1547
   


-- 
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@ratis.apache.org

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



[GitHub] [ratis] captainzmc closed pull request #617: RATIS-1547. Make GrpcClientProtocolService and log to public

Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #617:
URL: https://github.com/apache/ratis/pull/617


   


-- 
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@ratis.apache.org

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



[GitHub] [ratis] szetszwo commented on a change in pull request #617: RATIS-1547. Make GrpcClientProtocolService and log to public

Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #617:
URL: https://github.com/apache/ratis/pull/617#discussion_r818697351



##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcClientProtocolService.java
##########
@@ -48,8 +48,8 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
-class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
-  private static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
+public class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
+  public static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);

Review comment:
       @captainzmc , sorry that GrpcClientProtocolService is not a public API.  We should not change it for an Ozone unit test.  We should fix the test instead.




-- 
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@ratis.apache.org

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



[GitHub] [ratis] captainzmc commented on pull request #617: RATIS-1547. Make GrpcClientProtocolService and log to public

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #617:
URL: https://github.com/apache/ratis/pull/617#issuecomment-1058117693


   Thanks @szetszwo and @adoroszlai  for the suggestion.  Agree with you, let’s improve TestRatisPipelineLeader to avoid this error.


-- 
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@ratis.apache.org

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



[GitHub] [ratis] adoroszlai commented on a change in pull request #617: RATIS-1547. Make GrpcClientProtocolService and log to public

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #617:
URL: https://github.com/apache/ratis/pull/617#discussion_r818704477



##########
File path: ratis-grpc/src/main/java/org/apache/ratis/grpc/server/GrpcClientProtocolService.java
##########
@@ -48,8 +48,8 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
-class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
-  private static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
+public class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
+  public static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);

Review comment:
       I also encountered compile error while trying recent Ratis changes (RATIS-1515, etc.) with Ozone.  But I think exposing `LOG` as public only for Ozone test is bad practice.  We may be able to improve `TestRatisPipelineLeader` by using custom retry policy.




-- 
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@ratis.apache.org

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



[GitHub] [ratis] szetszwo commented on pull request #617: RATIS-1547. Make GrpcClientProtocolService and log to public

Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #617:
URL: https://github.com/apache/ratis/pull/617#issuecomment-1069420084


   @captainzmc , filed https://issues.apache.org/jira/browse/HDDS-6461 for updating Ratis snapshot version including fixing TestRatisPipelineLeader; see https://github.com/apache/ozone/pull/3205


-- 
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@ratis.apache.org

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