-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[7.x] [Snapshot Restore] fix snapshot and repository endpoints to align with ES API #60508
Conversation
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / kibana-oss-agent / Accessibility Tests.test/accessibility/apps/discover·ts.Discover Load a new search from the panelStandard Out
Stack Trace
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally and verified expected behavior of fetching all repositories, all snapshots, and individual snapshots. Code also looks good. I compared it to the reference PR and it looks like everything is mirrored.
Great catch @alisonelizabeth ! 👍 I wonder how we could avoid this (big) mistake. Let's discuss it at our next sync because it might/will happen again. I am thinking about the mappings editor and difference of logic How do we communicate in a central place apps that can be freely backported (no diffs between major versions) and apps that have different logic and thus the backport (maybe) needs to be manual. |
@sebelga I think an end-to-end test would have caught this, right? We could have tested the UI's dependency upon the ES APIs in question. By consuming the real ES APIs in 7.x, we would have seen the backport fail CI because the client would have expected a different API than what was provided. |
I think we could supplement these tests by also supporting both the 7.x and 8.0 forms in both The tests will alert us of a problem, but the problems will be much less likely to occur in the first place. I also like the consistent pattern of branching logic on the stack version. It gives us something reliable to grep for when looking for these types of divergences and it makes the divergence it very obvious to anyone who is skimming the code. |
I am not sure I follow you 😄 This is worth a zoom I guess. |
This PR fixes a regression caused by the 7.x backport PR of SR NP migration: #59520.
There are differences between
master
and7.x
branches for the snapshot and repository API endpoint handlers due to breaking changes in ES snapshot APIs. More details here: #39533.How to test