[GitHub] [druid] rohangarg commented on a change in pull request #12215: Fixing HttpEntity retry mechanism

[GitBox <gi...@apache.org>]

rohangarg commented on a change in pull request #12215:
URL: https://github.com/apache/druid/pull/12215#discussion_r796668598



##########
File path: core/src/main/java/org/apache/druid/data/input/impl/HttpEntity.java
##########
@@ -80,23 +80,30 @@ protected String getPath()
     return t -> t instanceof IOException;
   }
 
-  public static InputStream openInputStream(URI object, String userName, PasswordProvider passwordProvider, long offset)
-      throws IOException
-  {
-    final URLConnection urlConnection = object.toURL().openConnection();
+  private static void addAuthHeader(URLConnection urlConnection, String userName, PasswordProvider passwordProvider){
     if (!Strings.isNullOrEmpty(userName) && passwordProvider != null) {
       String userPass = userName + ":" + passwordProvider.getPassword();
       String basicAuthString = "Basic " + Base64.getEncoder().encodeToString(StringUtils.toUtf8(userPass));
       urlConnection.setRequestProperty("Authorization", basicAuthString);
     }
+  }
+
+  public static InputStream openInputStream(URI object, String userName, PasswordProvider passwordProvider, long offset)
+      throws IOException
+  {
+    final URLConnection urlConnection = object.toURL().openConnection();
+    addAuthHeader(urlConnection, userName, passwordProvider);
     final String acceptRanges = urlConnection.getHeaderField(HttpHeaders.ACCEPT_RANGES);
     final boolean withRanges = "bytes".equalsIgnoreCase(acceptRanges);
     if (withRanges && offset > 0) {
       // Set header for range request.
       // Since we need to set only the start offset, the header is "bytes=<range-start>-".
       // See https://tools.ietf.org/html/rfc7233#section-2.1
-      urlConnection.addRequestProperty(HttpHeaders.RANGE, StringUtils.format("bytes=%d-", offset));
-      return urlConnection.getInputStream();
+      urlConnection.getInputStream().close();

Review comment:
       A couple of doubts : 
   1. maybe we can use try-with-resources to manage the connection so that there's no leak incase of any exception
   2. is it possible to determine the `ACCEPT-RANGES` value from a `HEAD` call? If we can do so, then one positive is that we can complete the requests we do to the server. but i'm not sure if that's possible




-- 
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: commits-unsubscribe@druid.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org