saxenapranav commented on code in PR #6025:
URL: https://github.com/apache/hadoop/pull/6025#discussion_r1328475380
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/StorageInterfaceImpl.java:
##########
@@ -435,6 +456,24 @@ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions op
null, dstAccessCondition, options, opContext);
}
+ @Override
+ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions options,
+ OperationContext opContext, boolean
overwriteDestination, String eTag)
+ throws StorageException, URISyntaxException {
+ AccessCondition dstAccessCondition =
+ overwriteDestination
+ ? null
+ : AccessCondition.generateIfNotExistsCondition();
+ if (dstAccessCondition != null) {
Review Comment:
Better we check if etag == null. Agree sdk checks that before populating
header. But SDK can get change in future.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/StorageInterfaceImpl.java:
##########
@@ -315,6 +315,20 @@ public void delete(OperationContext opContext,
SelfRenewingLease lease)
null, opContext);
}
+ @Override
+ public void delete(OperationContext opContext, SelfRenewingLease lease,
String eTag)
+ throws StorageException {
+ AccessCondition accessCondition1 = getLeaseCondition(lease);
+ if (accessCondition1 != null && eTag != null) {
+ accessCondition1.setIfMatch(eTag);
Review Comment:
if lease condition is there, should etag check be there?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/StorageInterfaceImpl.java:
##########
@@ -435,6 +456,24 @@ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions op
null, dstAccessCondition, options, opContext);
}
+ @Override
+ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions options,
+ OperationContext opContext, boolean
overwriteDestination, String eTag)
+ throws StorageException, URISyntaxException {
+ AccessCondition dstAccessCondition =
+ overwriteDestination
+ ? null
+ : AccessCondition.generateIfNotExistsCondition();
+ if (dstAccessCondition != null) {
+ dstAccessCondition.setIfMatch(eTag);
Review Comment:
the object will have both if-none-match:* and if-match:ETAG.
We should not populate when overwrite condition is on.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/StorageInterfaceImpl.java:
##########
@@ -435,6 +456,24 @@ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions op
null, dstAccessCondition, options, opContext);
}
+ @Override
+ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions options,
Review Comment:
lets define what etag is this. Rename of as dstEtag would be good.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azure/AzureNativeFileSystemStore.java:
##########
@@ -2761,18 +2816,23 @@ public boolean delete(String key) throws IOException {
@Override
public void rename(String srcKey, String dstKey) throws IOException {
- rename(srcKey, dstKey, false, null, true);
+ rename(srcKey, dstKey, false, null, true, null);
+ }
+
+ @Override
+ public void rename(String srcKey, String dstKey, String eTag) throws
IOException {
+ rename(srcKey, dstKey, false, null, true, eTag);
Review Comment:
lets define what etag is this. is this etag on src or dst.
##########
hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azure/MockStorageInterface.java:
##########
@@ -450,6 +453,22 @@ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions op
// update azureNativeFileSystemStore.waitForCopyToComplete
}
+ @Override
+ public void startCopyFromBlob(CloudBlobWrapper sourceBlob,
BlobRequestOptions options,
+ OperationContext opContext, boolean
overwriteDestination, String eTag)
+ throws StorageException, URISyntaxException {
+ if (!overwriteDestination &&
backingStore.exists(convertUriToDecodedString(uri))) {
Review Comment:
why we need an overriden impl?
--
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]
