ahmarsuhail commented on code in PR #7011:
URL: https://github.com/apache/hadoop/pull/7011#discussion_r1732579400
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -608,6 +611,16 @@ private long putObject() throws IOException {
builder.putOptions,
false);
+ PutObjectRequest.Builder maybeModifiedPutIfAbsentRequest =
putObjectRequest.toBuilder();
+ Map<String, String> optionHeaders = builder.putOptions.getHeaders();
+
+ if (optionHeaders != null &&
optionHeaders.containsKey(IF_NONE_MATCH_HEADER)) {
+ maybeModifiedPutIfAbsentRequest.overrideConfiguration(
Review Comment:
as mentioned below as well, think we should upgrade SDK and then use the new
.ifNoneMatch().
Also I would recommend you move all of this logic into a new private method
in this class.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3ABlockOutputStream.java:
##########
@@ -608,6 +611,16 @@ private long putObject() throws IOException {
builder.putOptions,
false);
+ PutObjectRequest.Builder maybeModifiedPutIfAbsentRequest =
putObjectRequest.toBuilder();
+ Map<String, String> optionHeaders = builder.putOptions.getHeaders();
+
+ if (optionHeaders != null &&
optionHeaders.containsKey(IF_NONE_MATCH_HEADER)) {
+ maybeModifiedPutIfAbsentRequest.overrideConfiguration(
Review Comment:
actually, I would move this logic to RequestFactoryImpl. But you will need
to add in a flag to the method signature, "isConditonalPutEnabled", and only
add the if-none-match header in when sConditonalPutEnabled is true.
Basically, you only want to add ` if-none-match` when the PUT is coming
from S3ABlockOutputStream.close() and not from anywhere else.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/RequestFactoryImpl.java:
##########
@@ -517,12 +518,22 @@ public CreateMultipartUploadRequest.Builder
newMultipartUploadRequestBuilder(
public CompleteMultipartUploadRequest.Builder
newCompleteMultipartUploadRequestBuilder(
String destKey,
String uploadId,
- List<CompletedPart> partETags) {
+ List<CompletedPart> partETags,
+ PutObjectOptions putOptions) {
+
// a copy of the list is required, so that the AWS SDK doesn't
// attempt to sort an unmodifiable list.
- CompleteMultipartUploadRequest.Builder requestBuilder =
-
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
+ CompleteMultipartUploadRequest.Builder requestBuilder;
+ Map<String, String> optionHeaders = putOptions.getHeaders();
+
+ if (optionHeaders != null && optionHeaders.containsKey("If-None-Match")) {
+ requestBuilder =
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
+ .overrideConfiguration(override
->override.putHeader("If-None-Match", optionHeaders.get("If-None-Match")))
.multipartUpload(CompletedMultipartUpload.builder().parts(partETags).build());
+ } else {
+ requestBuilder =
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
Review Comment:
you don't need to duplicate all of this. just do:
```
requestBuilder =
CompleteMultipartUploadRequest.builder().bucket(bucket).key(destKey).uploadId(uploadId)
.multipartUpload(CompletedMultipartUpload.builder().parts(partETags).build());
if (optionHeaders != null && optionHeaders.containsKey("If-None-Match"))
{
requestBuilder =
CompleteMultipartUploadRequest.builder().overrideConfiguration(override
->override.putHeader("If-None-Match", optionHeaders.get("If-None-Match"));
}
```
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1390,6 +1390,13 @@ private Constants() {
*/
public static final String FS_S3A_CREATE_PERFORMANCE =
"fs.s3a.create.performance";
+ /**
+ * Flag for commit if none match.
+ * This can be set in the {code createFile()} builder.
+ * Value {@value}.
+ */
+ public static final String FS_S3A_CREATE_IF_NONE_MATCH =
"fs.s3a.create.header.If-None-Match";
Review Comment:
+1, should change to `fs.s3a.create.header.if.none.match`
--
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]
