BlobContainer's deleteBlob method should not be lenient

[abeyad]

Currently, several implementations of BlobContainer#deleteBlob(String blobName) are lenient - if the blob does not exist, it silently does nothing. Instead, we should throw an IOException if the blob does not exist. This is in accordance with BlobContainer's contract as stated in #18157.

Current implementations that do not conform (and silently do nothing if the blob does not exist):

1. FsBlobContainer
2. HdfsBlobContainer
3. S3BlobContainer
4. AzureBlobContainer

[gfyoung]

S3BlobContainerconforms here, and AzureBlobContainerconforms here.
The other two implementations do not conform though as you pointed out.

[abeyad]

@gfyoung Actually, the S3BlobContainer does not yet conform, because this line calls S3BlobStore.client()#deleteObject. The S3BlobStore.client() returns an instance of AmazonS3. If you look at the javadoc for AmazonS3#deleteObject, they say that the absence of the object will not throw an exception:

http://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/AmazonS3.html#deleteObject(java.lang.String,%20java.lang.String)

The exception handling line you linked to only gets triggered if there are actual errors during deleting the object in S3, not if the object itself does not exist.

I believe its the same with AzureBlobContainer.

Instead, in both S3 and Azure, we should first check for the presence of the object and if it does not exist, throw an IOException before deleting.

[jasontedor]

S3BlobContainerconforms here, and AzureBlobContainerconforms here.
The other two implementations do not conform though as you pointed out.

This is not correct. The documentation for AmazonS3#deleteObject specifies:

     * If attempting to delete an object that does not exist,
     * Amazon S3 returns
     * a success message instead of an error message.

It's not clear what the Azure service does as it's not specified in the docs, it would have to be tested.

[jasontedor]

Also, it's important to note that some exceptions that are being wrapped into IOExceptions probably should not be, for example, general client exceptions and URI syntax exceptions.

[gfyoung]

@abeyad : Hmmm...okay I see, so the correct fix would to call blobExists first before attempting the actual deletion.

@jasontedor : With regards to exceptions, where do we draw the line with regards to the statement here in the contract? I agree that URI syntax errors should be excluded, but others are not as clear in my mind (e.g. exceptions associated with Files.deleteIfExists).

Also, I do agree that testing needs to be done, but I'm not sure how to do it as I stated here in the PR.

[abeyad]

Hmmm...okay I see, so the correct fix would to call blobExists first before attempting the actual deletion.

@gfyoung yes that's correct

[tlrx]

It's not clear what the Azure service does as it's not specified in the docs, it would have to be tested.

Same for Google Cloud Storage, I don't see any documentation about what it does.