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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
