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

Fix: Update field names in /index/_recovery API response to snake_case for consistency #16541

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Aradhya2708
Copy link

@Aradhya2708 Aradhya2708 commented Nov 1, 2024

Description

This PR updates field names in the SnapshotRecoverySource response to follow snake_case, ensuring consistency across OpenSearch API responses. Specifically, it modifies the following fields:

  • isSearchableSnapshot -> is_searchable_snapshot
  • remoteStoreIndexShallowCopy -> remote_store_index_shallow_copy
  • sourceRemoteStoreRepository -> source_remote_store_repository
  • sourceRemoteTranslogRepository -> source_remote_translog_repository

This update aligns the naming conventions within the API, addressing an inconsistency in the /{index}/_recovery API response format.

Related Issues

Resolves #16334

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

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.

Updated field names in SnapshotRecoverySource to use snake_case for consistency across API responses:
- isSearchableSnapshot -> is_searchable_snapshot
- remoteStoreIndexShallowCopy -> remote_store_index_shallow_copy
- sourceRemoteStoreRepository -> source_remote_store_repository
- sourceRemoteTranslogRepository -> source_remote_translog_repository

This change aligns with OpenSearch API naming conventions.

Signed-off-by: Aradhya Mahajan <[email protected]>
Copy link
Contributor

github-actions bot commented Nov 1, 2024

✅ Gradle check result for aa9aeda: SUCCESS

Copy link

codecov bot commented Nov 1, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 72.10%. Comparing base (0363aa7) to head (aa9aeda).
Report is 59 commits behind head on main.

Files with missing lines Patch % Lines
...org/opensearch/cluster/routing/RecoverySource.java 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #16541      +/-   ##
============================================
+ Coverage     72.00%   72.10%   +0.10%     
- Complexity    65038    65080      +42     
============================================
  Files          5313     5313              
  Lines        303454   303454              
  Branches      43910    43910              
============================================
+ Hits         218510   218819     +309     
+ Misses        67040    66696     -344     
- Partials      17904    17939      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@dbwiddis dbwiddis 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 this fix, @Aradhya2708 ! A few things:

  1. Please add a CHANGELOG entry for this, in the "unreleased 3.0" section since it will not be backported.
  2. You'll need to update the spec at https://github.com/opensearch-project/opensearch-api-specification/blob/main/spec/schemas/indices.recovery.yaml for the 3.0 tests

@@ -415,10 +415,10 @@ public void addAdditionalFields(XContentBuilder builder, ToXContent.Params param
.field("version", version.toString())
.field("index", index.getName())
.field("restoreUUID", restoreUUID)
Copy link
Member

Choose a reason for hiding this comment

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

Should this one also be changed?

@owaiskazi19
Copy link
Member

@dbwiddis we might need BWC tests for this change?

@dbwiddis
Copy link
Member

dbwiddis commented Nov 5, 2024

@dbwiddis we might need BWC tests for this change?

It is by definition not backwards compatible. This is a 3.x-only change. It needs to be documented.

@msfroh
Copy link
Collaborator

msfroh commented Nov 5, 2024

  1. Please add a CHANGELOG entry for this, in the "unreleased 3.0" section since it will not be backported.

Note that you'll need to add the entry in https://github.com/opensearch-project/OpenSearch/blob/main/CHANGELOG-3.0.md.

(We split the 2.x and 3.0 changelogs a few months ago.)

@owaiskazi19 owaiskazi19 added the v3.0.0 Issues and PRs related to version 3.0.0 label Nov 6, 2024
@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Dec 7, 2024
@dblock
Copy link
Member

dblock commented Dec 9, 2024

Thanks @Aradhya2708!

  • This path doesn't seem to be covered by tests, it should, even if trivial.
  • Is there a way to add something to CI that prevents this problem in the future? Maybe tests that retrieve data inherit from a class that always checks for this type of thing?

@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers Other Storage:Snapshots v3.0.0 Issues and PRs related to version 3.0.0
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[BUG] The /{index}/_recovery API returns responses in a non-standard format(using camelCase).
5 participants