[GitHub] [druid] FrankChen021 commented on a diff in pull request #13195: follow RFC7232

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

FrankChen021 commented on code in PR #13195:
URL: https://github.com/apache/druid/pull/13195#discussion_r990879367


##########
extensions-core/s3-extensions/src/main/java/org/apache/druid/storage/s3/S3TaskLogs.java:
##########
@@ -95,9 +96,15 @@ private Optional<InputStream> streamTaskFile(final long offset, String taskKey)
         }
 
         final GetObjectRequest request = new GetObjectRequest(config.getS3Bucket(), taskKey)
-            .withMatchingETagConstraint("\"" + objectMetadata.getETag() + "\"")
+            .withMatchingETagConstraint(objectMetadata.getETag())
             .withRange(start, end);
 
+        if (objectMetadata.getETag() != null) {
+          if (!objectMetadata.getETag().startsWith("\"") && !objectMetadata.getETag().endsWith("\"")) {
+            request.setMatchingETagConstraints(Collections.singletonList("\"" + objectMetadata.getETag() + "\""));
+          }
+        }

Review Comment:
   We can put these lines to a helper method to check and add quotation marks for etag so that the code will be simplied as
   
   
   ```java
   final GetObjectRequest request = new GetObjectRequest(config.getS3Bucket(), taskKey)
               .withMatchingETagConstraint(ensureQuotated(objectMetadata.getETag()))
               .withRange(start, end);
   
   static String ensureQuotated(String eTag) {
      ...
   }
   ```
   
   And this help method can also be used by the test cases.
   
   For the test cases, we also need a test case that accept a etag without quotation and check the the etag in the final reuqest has quotation marks.



-- 
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