gaborgsomogyi commented on code in PR #23930:
URL: https://github.com/apache/flink/pull/23930#discussion_r1428109606


##########
flink-runtime/src/main/java/org/apache/flink/runtime/rest/RestClient.java:
##########
@@ -632,12 +633,16 @@ private static <P extends ResponseBody> 
CompletableFuture<P> parseResponse(
         CompletableFuture<P> responseFuture = new CompletableFuture<>();
         final JsonParser jsonParser = 
objectMapper.treeAsTokens(rawResponse.json);
         try {
+            if (responseType.getRawClass().equals(EmptyResponseBody.class)
+                    && !rawResponse.getJson().isEmpty()) {
+                throw new IOException("Expecting empty response but presumably 
error arrived");
+            }

Review Comment:
   This has been added since the approval to resolve the unit test issues 
[here](https://dev.azure.com/apache-flink/apache-flink/_build/results?buildId=55522&view=results).
 I think we need to revisit the approach.
   
   In short error parsing was triggered by failed response parsing without this 
change. With this change we're not blowing up when `EmptyResponseBody` is 
expected because the object mapper can read the nothing successfully. Because 
of this I've added the edge case here.
   
   Important to say that when the response contains fields and any of them are 
missing then the parsing throws exception so we're fine.
   
   The approach is negotiable because there are multiple options. I've tried 
many but this makes the most sense to me.



-- 
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: issues-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to