Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4358#discussion_r139956243
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/blob/BlobClient.java ---
    @@ -601,7 +564,39 @@ public void deleteInternal(@Nullable JobID jobId, 
BlobKey key) throws IOExceptio
        }
     
        /**
    -    * Uploads the JAR files to a {@link BlobServer} at the given address.
    +    * Reads the response from the input stream and throws in case of errors
    +    *
    +    * @param is
    +    *              stream to read from
    +    *
    +    * @return  <tt>true</tt> if the delete operation was successful at the 
{@link BlobServer};
    +    *          <tt>false</tt> otherwise
    +    *
    +    * @throws IOException
    +    *              if the server code throws an exception or if reading 
the response failed
    +    */
    +   private static boolean receiveAndCheckDeleteResponse(InputStream is) 
throws IOException {
    +           int response = is.read();
    +           if (response < 0) {
    +                   throw new EOFException("Premature end of response");
    +           }
    +           if (response == RETURN_ERROR) {
    +                   Throwable cause = readExceptionFromStream(is);
    +                   if (cause == null) {
    +                           return false;
    +                   } else {
    +                           throw new IOException("Server side error: " + 
cause.getMessage(), cause);
    --- End diff --
    
    I'm tempted to leave it that way not only because previous code followed 
that pattern (as in `BlobClient#receiveAndCheckGetResponse()`) but also because 
it is nice to have a brief failure message with some details without having to 
dig through the stack trace.


---

Reply via email to