Take buildTableCreationResponse out of safelyExecutes #3161
Conversation
devinrsmith
added
clean up
NoDocumentationNeeded
NoReleaseNotesNeeded
| responseObserver.onNext(ExportUtil.buildTableCreationResponse(request, (Table) obj)); | ||
| responseObserver.onCompleted(); |
There was a problem hiding this comment.
I'm assuming it was on oversight this wasn't in a safelyExecute?
There was a problem hiding this comment.
It doesn't hurt - the risk is that some work after the call might fail to be performed, but since this is a nonExport() submit, nothing need to be returned and stored for future ticket references.
So we're just mitigating some log messages.
I don't love overly defensive calls like this - but I also really don't love the layers we've had to stack on gRPC to make it usable at all, so I'm good with it.
| responseObserver.onNext(ExportUtil.buildTableCreationResponse(request, (Table) obj)); | ||
| responseObserver.onCompleted(); |
There was a problem hiding this comment.
It doesn't hurt - the risk is that some work after the call might fail to be performed, but since this is a nonExport() submit, nothing need to be returned and stored for future ticket references.
So we're just mitigating some log messages.
I don't love overly defensive calls like this - but I also really don't love the layers we've had to stack on gRPC to make it usable at all, so I'm good with it.
| .setSuccess(false) | ||
| .setErrorInfo(errorInfo) | ||
| .build(); | ||
| safelyExecuteLocked(responseObserver, () -> responseObserver.onNext(response)); |
There was a problem hiding this comment.
We've got so many of these... I almost want to just have a wrapper/subtype of StreamObserver that does these for us, like onCompleted(Response) to do both onNext and onCompleted safely+locked, and auto-wrap both onNext and onCompleted with the lock and safe wrapper.
No description provided.