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

[Snapshot + Restore] Fix error handling for repositories API #103723

Merged

Conversation

alisonelizabeth
Copy link
Contributor

@alisonelizabeth alisonelizabeth commented Jun 29, 2021

Fixes #103673

This appears to be a regression that only made it to master, although it will now need to be backported to align with the work done in #103331. There was a test that checked it, but it was not set up correctly and was falsely passing.

To manually test, you can stub the ES snapshots API request so that it will fail (quickest/easiest), or you can set up a failed repository with snapshots to trigger an actual error. This involves manually altering the index-N file in your local repository so that it becomes invalid. The repository details panel should still show, but without the snapshot count.

Screen Shot 2021-06-29 at 1 23 41 PM

@alisonelizabeth alisonelizabeth added v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more release_note:skip Skip the PR/issue when compiling release notes Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI v7.14.0 labels Jun 29, 2021
@alisonelizabeth alisonelizabeth marked this pull request as ready for review June 29, 2021 20:20
@alisonelizabeth alisonelizabeth requested a review from a team as a code owner June 29, 2021 20:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-stack-management (Team:Stack Management)

@alisonelizabeth alisonelizabeth requested a review from sebelga June 29, 2021 20:21
Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Tested locally, code LGTM!

import type {
SnapshotGetRepositoryResponse,
SnapshotRepositorySettings,
} from '@elastic/elasticsearch/api/types';
Copy link
Contributor

Choose a reason for hiding this comment

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

+++++++++++++++++++++++++++

@cjcenizal cjcenizal added the auto-backport Deprecated - use backport:version if exact versions are needed label Jun 30, 2021
@cjcenizal
Copy link
Contributor

Merging now to make FF

@cjcenizal cjcenizal merged commit 57a9121 into elastic:master Jun 30, 2021
@kibanamachine
Copy link
Contributor

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 103723

@alisonelizabeth alisonelizabeth deleted the bugfix/sr_repo_error_handling branch June 30, 2021 00:35
@alisonelizabeth
Copy link
Contributor Author

Thanks @cjcenizal!

Re: backport failure. I think we might need to wait for #103693 to be merged into 7.x first. I was hitting issues with CI earlier, but hopefully this next run should be green 🤞 .

@alisonelizabeth
Copy link
Contributor Author

Backport PR: #103814

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Snapshot and Restore Elasticsearch snapshots and repositories UI release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Snapshot + Restore] Repository API does not handle snapshot error
4 participants