phet commented on code in PR #4076:
URL: https://github.com/apache/gobblin/pull/4076#discussion_r1847077237
##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/modules/restli/FlowConfigsV2ResourceHandler.java:
##########
@@ -256,7 +257,10 @@ public CreateKVResponse<ComplexResourceKey<FlowId,
FlowStatusId>, FlowConfig> cr
responseMap = this.flowCatalog.put(flowSpec, true);
} catch (QuotaExceededException e) {
throw new RestLiServiceException(HttpStatus.S_503_SERVICE_UNAVAILABLE,
e.getMessage());
- } catch (Throwable e) {
+ } catch(LeaseUnavailableException e){
+ throw new RestLiServiceException(HttpStatus.S_409_CONFLICT,
e.getMessage());
Review Comment:
usually exception messages are designed for logging, more than for end-user
consumption, so probably not appropriate to blindly return that. (it's
sometimes done for a 5xx error, as above... but even that can be inadvisable.)
anyway, the 409 above might offer a better template:
```
return new CreateKVResponse<>(new
RestLiServiceException(HttpStatus.S_409_CONFLICT,
"FlowSpec with URI " + flowSpec.getUri() + " was launched less than N
secs ago, no action will be taken"));
```
(to provide N we may wish to tunnel the value of epsilon... or at least how
many secs remain before a subsequent launch would be possible)
also: when do we want to `return` (as that 409 above does), vs. `throw`?
--
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: dev-unsubscr...@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
