-
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
[S&R] Support data streams #68078
[S&R] Support data streams #68078
Conversation
The server endpoint for policies now returns data streams and filters out backing indices from the indices array it returned previously
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
Given the contraints on working with data streams and indices in policies at the moment the simplest implementation is to just include data streams with indices and have the user select them there for now. The way snapshotting behaviour is currently implemented relies entirely on what is specified inside of "indices", this is also where data streams must be placed. This unfortunately means that capture patterns defined in indices will capture entire data streams too.
@elasticmachine merge upstream |
- added spacer between title and data streams callout - added copy to the restore settings tab to indicate that settings also apply to backing indices
@elasticmachine merge upstream |
Thanks for the review @jrodewig , @gchaps and @cjcenizal . I think I have addressed all of your points, it would be great to get another pass for the copy that I added for renaming in the restore flow: @cjcenizal I think I have addressed most of your points except for your last one:
I think in this case only indices are available which makes this view correct. It is inconsistent with the other views, but it does not look like the data streams are returned for us to display here unfortunately - there may be some way to get this that I am not aware of. I will as ES side about this. Otherwise, would you mind taking another look? |
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.
Left a couple of non-blocking nits. LGTM.
...steps/step_settings/fields/indices_and_data_streams_field/indices_and_data_streams_field.tsx
Outdated
Show resolved
Hide resolved
body: () => ( | ||
<FormattedMessage | ||
id="xpack.snapshotRestore.restoreForm.dataStreamsWarningCallOut.body" | ||
defaultMessage="Each data stream requires a matching index template. Please ensure any restored data streams have a matching index template. You can restore index templates by restoring the global cluster state. However, this may overwrite existing templates, cluster settings, ingest pipelines, and lifecycle policies. {learnMoreLink} about restoring snapshots that contain data streams." |
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.
👍
...snapshot_restore/public/application/components/restore_snapshot_form/steps/step_settings.tsx
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
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.
Code looks great! Thanks for making those copy changes. I had a number of small suggestions but nothing blocking.
x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/mocks.tsx
Show resolved
Hide resolved
x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/restore_snapshot.helpers.ts
Show resolved
Hide resolved
x-pack/plugins/snapshot_restore/__jest__/client_integration/helpers/setup_environment.tsx
Show resolved
Hide resolved
x-pack/plugins/snapshot_restore/common/lib/is_data_stream_backing_index.ts
Show resolved
Hide resolved
...ot_restore/public/application/components/collapsible_lists/collapsible_data_streams_list.tsx
Show resolved
Hide resolved
@@ -73,8 +73,8 @@ export const RestoreSnapshotStepReview: React.FunctionComponent<StepProps> = ({ | |||
<EuiDescriptionList textStyle="reverse"> | |||
<EuiDescriptionListTitle> | |||
<FormattedMessage | |||
id="xpack.snapshotRestore.restoreForm.stepReview.summaryTab.indicesLabel" | |||
defaultMessage="Indices" | |||
id="xpack.snapshotRestore.restoreForm.stepReview.summaryTab.dataStreamsAndIndicesLabel" |
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.
I think we should keep this ID the same, to make it easier on the translators. We also want to uncouple the ID from the content so we don't need to change the id every time the label content changes, e.g. if we decide to change this to "Index abstractions" or even back to "Indices" in the future. This suggestion applies to other i18n IDs in this PR if there are similar changes.
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.
Hm, I was actually thinking this would make the new translation work more discoverable for the translators. They will need to revisit these translations at any rate and I think the JSON doc is pretty much auto-generated.
I'd expect at some point for a release after 7.9 we will revisit these, hopefully with data streams and indices separated. For now I think these are also nice markers in the code for where we will need to change future copy again.
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.
Makes sense! Would you mind pinging the i18n team about this? You raise good points, I'm just curious what the domain owners advise.
...snapshot_restore/public/application/components/restore_snapshot_form/steps/step_settings.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/snapshot_restore/public/application/services/http/policy_requests.ts
Show resolved
Hide resolved
@elasticmachine merge upstream |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
* Sort endpoint responses into indices and datastreams The server endpoint for policies now returns data streams and filters out backing indices from the indices array it returned previously * Refactor indices switch and field out of the step settings file * Fix indices field form behaviour * WiP on UI. Added the second table per mockup for add and edit. * add support for creating a policy that backs up data streams end to end * wip on restore flow - added data streams to server response * add logic for detecting whether an index is part of a data stream * fix public jest tests * fix server side jest tests * pivot to different solution in UI while we do not have data streams nicely separated * added data stream to snapshot summary details * move the data streams badge file closer to where it used * add data stream badge when restoring snapshots too * update restore copy * fix pattern specification in indices and data streams field * first iteration of complete policy UX * First iteration that is ready for review Given the contraints on working with data streams and indices in policies at the moment the simplest implementation is to just include data streams with indices and have the user select them there for now. The way snapshotting behaviour is currently implemented relies entirely on what is specified inside of "indices", this is also where data streams must be placed. This unfortunately means that capture patterns defined in indices will capture entire data streams too. * delete unused import * fix type issue in tests * added logic for rendering out previous selection as custom pattern * refactor indices fields to make component smaller * added CIT for data streams badge * Data streams > indices * updates to relevant pieces of copy * more copy updates * fix types and remove unused import * removed backing indices from restore view * Added data stream restore warning message * restore CITs * first round of copy feedback * refactor help text to provide clearer feedback, for both restore and policy forms * Restore updates - added spacer between title and data streams callout - added copy to the restore settings tab to indicate that settings also apply to backing indices * further copy refinements * second round of copy feedback * fix i18n * added comment to mock * line spacing fixes and created issue for tracking backing index discovery in snaphots * refactor collapsible list logic and tests * refactor editing managed policy check * refactor copy to be clearer about pluralisation of data streams * refactor file structure in components for data stream badge * added tests for indices and data streams field helper * refactored types and fixed i18n id per guidelines Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* master: Update known-plugins.asciidoc (elastic#69370) [ML] Anomaly Explorer swim lane pagination (elastic#70063) [Ingest Manager] Update asset paths to use _ instead of - (elastic#70320) Fix discover, tsvb and Lens chart theming issues (elastic#69695) Allow Saved Object type mappings to set a field's `doc_values` property (elastic#70433) [S&R] Support data streams (elastic#68078) [Maps] Add styling and tooltip support to mapbox mvt vector tile sources (elastic#64488) redirect to default app if hash can not be forwarded (elastic#70417) [APM] Don't fetch dynamic index pattern in setupRequest (elastic#70308) [Security_Solution][Endpoint] Leveraging msearch and ancestry array for resolver (elastic#70134) Update docs for api authentication usage (elastic#66819) chore(NA): disable alerts_detection_rules cypress suites (elastic#70577) add getVisibleTypes API to SO type registry (elastic#70559)
* Sort endpoint responses into indices and datastreams The server endpoint for policies now returns data streams and filters out backing indices from the indices array it returned previously * Refactor indices switch and field out of the step settings file * Fix indices field form behaviour * WiP on UI. Added the second table per mockup for add and edit. * add support for creating a policy that backs up data streams end to end * wip on restore flow - added data streams to server response * add logic for detecting whether an index is part of a data stream * fix public jest tests * fix server side jest tests * pivot to different solution in UI while we do not have data streams nicely separated * added data stream to snapshot summary details * move the data streams badge file closer to where it used * add data stream badge when restoring snapshots too * update restore copy * fix pattern specification in indices and data streams field * first iteration of complete policy UX * First iteration that is ready for review Given the contraints on working with data streams and indices in policies at the moment the simplest implementation is to just include data streams with indices and have the user select them there for now. The way snapshotting behaviour is currently implemented relies entirely on what is specified inside of "indices", this is also where data streams must be placed. This unfortunately means that capture patterns defined in indices will capture entire data streams too. * delete unused import * fix type issue in tests * added logic for rendering out previous selection as custom pattern * refactor indices fields to make component smaller * added CIT for data streams badge * Data streams > indices * updates to relevant pieces of copy * more copy updates * fix types and remove unused import * removed backing indices from restore view * Added data stream restore warning message * restore CITs * first round of copy feedback * refactor help text to provide clearer feedback, for both restore and policy forms * Restore updates - added spacer between title and data streams callout - added copy to the restore settings tab to indicate that settings also apply to backing indices * further copy refinements * second round of copy feedback * fix i18n * added comment to mock * line spacing fixes and created issue for tracking backing index discovery in snaphots * refactor collapsible list logic and tests * refactor editing managed policy check * refactor copy to be clearer about pluralisation of data streams * refactor file structure in components for data stream badge * added tests for indices and data streams field helper * refactored types and fixed i18n id per guidelines Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Summary
Update SLM management to support data streams. Rest of PR description WIP.
See #65132 (comment)
How to test
yarn es snapshot -E path.repo=/tmp/es-snaps
)Create a data stream
Create an index template
Create a data stream
Checklist
Delete any items that are not applicable to this PR.
Screenshots
Data streams and indices
Snapshot details view
Restoring a snapshot