hillsp commented on a change in pull request #1945: Grpc RpcHandler: Fix finish
before start behavior
URL:
https://github.com/apache/incubator-pagespeed-mod/pull/1945#discussion_r319573793
##########
File path: pagespeed/controller/rpc_handler.h
##########
@@ -196,6 +196,10 @@ void RpcHandler<AsyncService, RequestT,
ResponseT>::InitDone(RefPtrT ref) {
// Function).
// Note that the status from our Finish() call does not make it to the
// client in this case.
+ ::grpc::Status status(::grpc::StatusCode::FAILED_PRECONDITION, "not
started");
Review comment:
I think CANCELLED is the right thing here, not FAILED_PRECONDITION.
FAILED_PRECONDITION is along the lines of "The system isn't in the right state
to perform your request" whereas CANCELLED is more like "I decided to not
perform your request for some reason". Most importantly, FAILED_PRECONDITION
usually means "you need to do something different or this won't work" whereas
CANCELLED is something you'd expect a retry to succeed. I also like that
CANCELLED is what it used to return, so it's 100% backwards compatible.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
