Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3866112900 > +1 the change looks good. @szetszwo Thanks for reviewing the code! -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
szetszwo merged PR #1342: URL: https://github.com/apache/ratis/pull/1342 -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3865866381 > > Let's move the try-catch from the sendRequest(request) to sendRequest(request, proxy). Then, the code is simpler; See https://issues.apache.org/jira/secure/attachment/13080704/1342_review.patch > > @slfan1989 , Have you looked at the comment above? @szetszwo Thank you for your suggestions! I’m updating this PR based on your feedback and will update it soon. -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
szetszwo commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3865854200 > Let's move the try-catch from the sendRequest(request) to sendRequest(request, proxy). Then, the code is simpler; See https://issues.apache.org/jira/secure/attachment/13080704/1342_review.patch @slfan1989 , Have you looked at the comment above? -- 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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on code in PR #1342:
URL: https://github.com/apache/ratis/pull/1342#discussion_r2778271182
##
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientRpc.java:
##
@@ -164,12 +176,35 @@ public void onCompleted() {
}
}
});
+replyFuture.whenComplete((reply, exception) -> {
+ if (replyFuture.isCancelled()) {
+requestObserver.onError(Status.CANCELLED
+.withDescription(clientId + ": request #" + request.getCallId() +
" cancelled")
+.asRuntimeException());
Review Comment:
Thank you so much for your suggestion! I’ve already updated the code.
--
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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
szetszwo commented on code in PR #1342:
URL: https://github.com/apache/ratis/pull/1342#discussion_r2778149777
##
ratis-grpc/src/main/java/org/apache/ratis/grpc/client/GrpcClientRpc.java:
##
@@ -164,12 +176,35 @@ public void onCompleted() {
}
}
});
+replyFuture.whenComplete((reply, exception) -> {
+ if (replyFuture.isCancelled()) {
+requestObserver.onError(Status.CANCELLED
+.withDescription(clientId + ": request #" + request.getCallId() +
" cancelled")
+.asRuntimeException());
Review Comment:
Let's use asException() instead of asRuntimeException().
--
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]
Re: [PR] RATIS-2400. Support timeout and interrupt handling in GrpcClientRpc. [ratis]
slfan1989 commented on PR #1342: URL: https://github.com/apache/ratis/pull/1342#issuecomment-3863912156 @szetszwo Could you please review this PR? Thank you very much! -- 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]
