Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Dec 8, 2017

This pull request changes the S3BlobContainer.blobExists() method implementation to make it use the AmazonS3.doesObjectExist() method instead of AmazonS3.getObjectMetadata(). The AmazonS3 implementation takes care of catching any thrown AmazonS3Exception and compares its response code with 404, returning false (object does not exist) or lets the exception be propagated.

The pull request also:

  • removes some duplicated code
  • removes the "snapshot is done" log trace in favour of a "snapshot is completed with state x" located right after the snapshot is effectively finalized in the repository

@tlrx tlrx added :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs review v6.0.2 v6.2.0 v7.0.0 labels Dec 8, 2017
@tlrx tlrx requested a review from imotov December 8, 2017 15:19
Copy link
Contributor

@imotov imotov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice cleanup! Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename it into deleteSnapshotBlobIgnoringErrors while you are at it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tlrx tlrx force-pushed the use-does-object-exist branch from f3188cf to 00e76a1 Compare December 11, 2017 09:56
@tlrx tlrx merged commit f27cb96 into elastic:master Dec 12, 2017
tlrx added a commit that referenced this pull request Dec 12, 2017
This pull request changes the S3BlobContainer.blobExists() method implementation 
to make it use the AmazonS3.doesObjectExist() method instead of 
AmazonS3.getObjectMetadata(). The AmazonS3 implementation takes care of 
catching any thrown AmazonS3Exception and compares its response code with 404, 
returning false (object does not exist) or lets the exception be propagated.
tlrx added a commit that referenced this pull request Dec 12, 2017
This pull request changes the S3BlobContainer.blobExists() method implementation
to make it use the AmazonS3.doesObjectExist() method instead of
AmazonS3.getObjectMetadata(). The AmazonS3 implementation takes care of
catching any thrown AmazonS3Exception and compares its response code with 404,
returning false (object does not exist) or lets the exception be propagated.
@tlrx tlrx added the v6.1.1 label Dec 12, 2017
tlrx added a commit that referenced this pull request Dec 12, 2017
This pull request changes the S3BlobContainer.blobExists() method implementation
to make it use the AmazonS3.doesObjectExist() method instead of
AmazonS3.getObjectMetadata(). The AmazonS3 implementation takes care of
catching any thrown AmazonS3Exception and compares its response code with 404,
returning false (object does not exist) or lets the exception be propagated.
@tlrx tlrx deleted the use-does-object-exist branch December 12, 2017 08:43
@tlrx
Copy link
Member Author

tlrx commented Dec 12, 2017

Thanks @imotov !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants