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.

Sign up for GitHub

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

Jump to bottom

Add spec for new rename_alias_pattern and rename_alias_replacement parameters to snapshot restore #615

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add spec for new rename_alias_pattern and rename_alias_replacement parameters to snapshot restore #615

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1472de6
Add spec for new rename_alias_pattern and rename_alias_replacement pa…
mispencer Oct 16, 2024
b18145d
Lint fixes
mispencer Oct 16, 2024
fccd6a5
Update CHANGELOG
mispencer Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions spec/namespaces/snapshot.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,10 @@ components:
type: string
rename_replacement:
type: string
rename_alias_pattern:
type: string
rename_alias_replacement:
type: string
description: Details of what to restore
responses:
snapshot.cleanup_repository@200:
Expand Down
57 changes: 56 additions & 1 deletion tests/snapshot/snapshot/restore.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@
- path: /books
method: DELETE
status: [200, 404]
- path: /stories
method: DELETE
status: [200, 404]
- path: /new_stories
Copy link
Member

Choose a reason for hiding this comment

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

We should call it something meaningful, maybe tales?

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean new_stories? This is the restored-with-new-name index. I changed it to renamed_stories.

Copy link
Member

Choose a reason for hiding this comment

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

I just don't love names like new_xyz or xyz_1 because we plan to use these samples to generate documentation. It's better with names like "movies" or "films", so instead of "new_stories" maybe we can use something semantically interesting like "tales"?

Copy link
Author

Choose a reason for hiding this comment

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

This name is for the rename parameters for this API. An entirely new name like that would not fit the most common use case of those parameters, which is to restore a copy of existing index with a new, non-conflicting name. So, even for documentation, I think a prefixed name like currently is more suitable. The current documentation of restore supports this - it has an example of a prefixed name, recovered-logs, and a suffixed name, opendistro-reports-definitions_restored, but no example of a entirely new name for renamed indexes.

Copy link
Member

@dblock dblock Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Understood, thanks. Maybe xyz-restored would make more semantic sense?

Take this opportunity to look at these restore tests and see if better names than "my-xyz" can be used. Think of what you'd want to read in the docs. NBD, but much appreciated.

method: DELETE
status: [200, 404]
prologues:
- path: /_snapshot/{repository}
method: PUT
Expand All @@ -33,6 +39,16 @@
method: PUT
- path: /books
method: PUT
- path: /stories
method: PUT
- path: /_aliases
method: POST
request:
payload:
actions:
- add:

Check failure on line 49 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Expected indentation of 10 spaces but found 8 spaces
index: "stories"

Check failure on line 50 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Expected indentation of 14 spaces but found 12 spaces

Check failure on line 50 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Must use plain style scalar

Check failure on line 50 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Strings must use singlequote
alias: "stories_alias"

Check failure on line 51 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Expected indentation of 14 spaces but found 12 spaces

Check failure on line 51 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Must use plain style scalar

Check failure on line 51 in tests/snapshot/snapshot/restore.yaml

View workflow job for this annotation

GitHub Actions / lint 

Strings must use singlequote
- path: /_snapshot/{repository}/{snapshot}
method: PUT
parameters:
Expand All @@ -44,6 +60,7 @@
indices:
- books
- movies
- stories
ignore_unavailable: true
include_global_state: false
partial: true
Expand All @@ -53,6 +70,9 @@
- path: /books
method: DELETE
status: [200, 404]
- path: /stories
method: DELETE
status: [200, 404]
chapters:
- synopsis: Restore snapshot with wait_for_completion true.
path: /_snapshot/{repository}/{snapshot}/_restore
Expand Down Expand Up @@ -98,4 +118,39 @@
- stage: DONE
type: SNAPSHOT
retry:
count: 3
count: 3
- synopsis: Restore snapshot with rename_pattern and rename_replacement.
path: /_snapshot/{repository}/{snapshot}/_restore
method: POST
parameters:
repository: my-fs-repository
snapshot: my-test-snapshot
wait_for_completion: true
request:
payload:
indices: stories
rename_pattern: stories
rename_replacement: new_stories
response:
status: 200
payload:
snapshot:
snapshot: my-test-snapshot
- synopsis: Restore snapshot with rename_alias_pattern and rename_alias_replacement.
path: /_snapshot/{repository}/{snapshot}/_restore
method: POST
parameters:
repository: my-fs-repository
snapshot: my-test-snapshot
wait_for_completion: true
request:
payload:
indices: stories
include_aliases: true
rename_alias_pattern: stories
rename_alias_replacement: new_stories
response:
status: 200
payload:
snapshot:
snapshot: my-test-snapshot
Loading