Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds ignoreUnavailable option to the snapshot status API #20066

Merged
merged 4 commits into from
Aug 19, 2016

Conversation

abeyad
Copy link

@abeyad abeyad commented Aug 18, 2016

Adds ignoreUnavailable to the snapshot status API to be consistant
with the get snapshots API which has a similar parameter. If
ignoreUnavailable is set to true, then the snapshot status request
will ignore any snapshots that were not found in the repository,
instead of throwing a SnapshotMissingException.

Closes #18522

with the get snapshots API which has a similar parameter. If
ignoreUnavailable is set to true, then the snapshot status request
will ignore any snapshots that were not found in the repository,
instead of throwing a SnapshotMissingException.

Closes elastic#18522
@abeyad abeyad added >enhancement review :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v5.0.0-beta1 labels Aug 18, 2016
* Set to <code>true</code> to ignore unavailable snapshots, instead of throwing an exception.
* Defaults to <code>false</code>, which means unavailable snapshots cause an exception to be thrown.
*
* @param ignoreUnavailable whether to ignore unavailable snapshots
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the @param or @return adds anything to the information. Personally I wouldn't add them. You can do what you want. I just wanted to let you know its ok (by me) not to add them if they don't make the API more clear.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, I usually don't add them but in this case I did to be consistent with the rest of the class' javadocs

@nik9000
Copy link
Member

nik9000 commented Aug 19, 2016

Left a bunch of small stuff.

@abeyad
Copy link
Author

abeyad commented Aug 19, 2016

@nik9000 I pushed de543bd to address your feedback

- is_true: snapshots

---
teardown:
Copy link
Member

Choose a reason for hiding this comment

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

I'd stick this up at the top near the setup so they are together. Easier to see that they are acting together that way.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@nik9000
Copy link
Member

nik9000 commented Aug 19, 2016

Left one final minor things. LGTM otherwise, merge when ready!

@abeyad
Copy link
Author

abeyad commented Aug 19, 2016

@nik9000 thanks for the review!

@abeyad abeyad merged commit 1c9b64e into elastic:master Aug 19, 2016
@abeyad abeyad deleted the snapshot-status-ignore-unavailable branch August 19, 2016 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement v5.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants