[
https://issues.apache.org/jira/browse/GOBBLIN-2089?focusedWorklogId=923839&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-923839
]
ASF GitHub Bot logged work on GOBBLIN-2089:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 18/Jun/24 07:49
Start Date: 18/Jun/24 07:49
Worklog Time Spent: 10m
Work Description: 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();
```
Issue Time Tracking
-------------------
Worklog Id: (was: 923839)
Time Spent: 20m (was: 10m)
> Fix /flowexecutions KILL and RESUME action to return HTTP 404 when flow
> execution doesn't exist
> -----------------------------------------------------------------------------------------------
>
> Key: GOBBLIN-2089
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2089
> Project: Apache Gobblin
> Issue Type: Improvement
> Components: gobblin-restli
> Reporter: Abhishek Jain
> Assignee: Hung Tran
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> When resume or killed is called for a FlowExecution which doesn't exist, we
> return 200 HttpStatus code currently.
> We should return 404 in such scenarios.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
