phet commented on code in PR #3975:
URL: https://github.com/apache/gobblin/pull/3975#discussion_r1643935388
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/restli/GobblinServiceFlowExecutionResourceHandlerWithWarmStandby.java:
##########
@@ -56,13 +56,13 @@ public
GobblinServiceFlowExecutionResourceHandlerWithWarmStandby(FlowExecutionRe
@Override
public void resume(ComplexResourceKey<FlowStatusId, EmptyRecord> key) {
- FlowStatusId id = key.getKey();
+ FlowStatusId id = this.get(key).getId();
Review Comment:
great how easy this fix has proven to be!
...still it may be too subtle for maintainers, who might:
1. presume `this.get()` is actually necessary for getting the
`FlowStatusId`, as opposed to an optional pre-check
2. not realize that the pre-check for existence is actually happening
hence, I'd suggest a comment, and potentially layout like:
```
this.get(key); // pre-check to throw `HttpStatus.S_404_NOT_FOUND`, instead
of blindly proceeding
FlowStatusId id = key.getKey();
```
--
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]
