Skip to content

Add support for convert_index_to_remote action#1406

Open
ztomaszewska wants to merge 3 commits into
opensearch-project:mainfrom
ztomaszewska:searchable-snapshot
Open

Add support for convert_index_to_remote action#1406
ztomaszewska wants to merge 3 commits into
opensearch-project:mainfrom
ztomaszewska:searchable-snapshot

Conversation

@ztomaszewska
Copy link
Copy Markdown

Description

This PR will add support for the Convert Index To Remote action in the ISM plugin.
It's a continuation of #1319 that has been hanging for quite some time now.

Issues Resolved

#1318

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link
Copy Markdown
Member

@shiv0408 shiv0408 left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up! The rename_pattern addition looks good. One issue before this can merge:

rename_pattern is typed as required in the interface, but the backend only serializes it when it differs from the default, that means existing policies created via the API won't have this field in their JSON. When the UI loads such a policy, rename_pattern will be undefined, causing isValid() to return false and the input to render undefined.

It should be marked optional in the interface:

  rename_pattern?: string;

And the form value should fall back to the backend default when absent:

value={(action.action as ConvertIndexToRemoteAction).convert_index_to_remote.rename_pattern ?? "$1_remote"}

And isValid() should not require it since the backend has a safe default:

isValid = () => {
  return !!this.action.convert_index_to_remote.snapshot && !!this.action.convert_index_to_remote.repository;
};

There's an existing test pattern to follow in public/pages/VisualCreatePolicy/components/UIActions/AliasUIAction/AliasUIAction.test.tsx, specifically the "renders with pre-defined actions" test. Please add a similar test case for ConvertIndexToRemoteUIAction that covers loading an action without rename_pattern to verify the field renders correctly with the default and isValid() still returns true.

tomsnuvv and others added 3 commits May 5, 2026 15:56
- Add new ConvertIndexToRemote UI component
- Correct tests

Signed-off-by: Tom Snuverink <tom_snuv@hotmail.com>
Signed-off-by: Zuzanna Tomaszewska <zuzanna.tomaszewska@sap.com>
Signed-off-by: Zuzanna Tomaszewska <zuzanna.tomaszewska@sap.com>
@ztomaszewska ztomaszewska force-pushed the searchable-snapshot branch from ff49513 to 1fcd99e Compare May 6, 2026 13:12
@ztomaszewska
Copy link
Copy Markdown
Author

ztomaszewska commented May 6, 2026

@shiv0408 thank you for review! Please take a look at my fixes :)

Also I found the other PR(#1373) regarding this Convert Index To Remote action. I don't know for which one we want to go.

Also, can I be so bold as to ask you to take a look at my other PR (#1407) if you have some spare time? :)

@ztomaszewska ztomaszewska requested a review from shiv0408 May 6, 2026 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants