Skip to content

Conversation

ste93cry
Copy link
Contributor

Description

The changes in this merge request focus on adding support for refreshing the search analyzers to the client of the ISM plugin.

Issues Resolved

Closes #678

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

codecov bot commented Mar 25, 2025

Codecov Report

❌ Patch coverage is 72.72727% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.37%. Comparing base (06a6dc8) to head (9d752b2).
⚠️ Report is 118 commits behind head on main.

Files with missing lines Patch % Lines
plugins/ism/api_refresh_search_analyzers-params.go 25.00% 8 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #686      +/-   ##
==========================================
+ Coverage   57.29%   60.37%   +3.08%     
==========================================
  Files         315      379      +64     
  Lines        9823    11738    +1915     
==========================================
+ Hits         5628     7087    +1459     
- Misses       2902     3212     +310     
- Partials     1293     1439     +146     
Flag Coverage Δ
integration 52.91% <72.72%> (+2.07%) ⬆️
unit 14.92% <ø> (+2.07%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
plugins/ism/api_refresh_search_analyzers.go 100.00% <100.00%> (ø)
plugins/ism/api_refresh_search_analyzers-params.go 25.00% <25.00%> (ø)

... and 112 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ste93cry ste93cry force-pushed the support-reload-search-analyzers-api-endpoint branch 3 times, most recently from 75b74fe to 3e4dd8c Compare March 25, 2025 23:58
@ste93cry
Copy link
Contributor Author

Test failures look unrelated to my changes. I think integration tests are a bit flaky, but I have no permission to restart the failed jobs

@ste93cry
Copy link
Contributor Author

ste93cry commented Oct 5, 2025

@dblock Hey, this pull request has been waiting for a bit. Can I have some feedback on this? I would love to see it merged and released 😃

Copy link
Collaborator

@Jakob3xD Jakob3xD left a comment

Choose a reason for hiding this comment

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

LGTM
Only the naming of Reload should be changed to Refresh

@ste93cry ste93cry force-pushed the support-reload-search-analyzers-api-endpoint branch from 3e4dd8c to 5824c4c Compare October 6, 2025 09:59
@ste93cry
Copy link
Contributor Author

ste93cry commented Oct 6, 2025

I pushed the requested changes, but I'm not sure what's going on with the linter. To me, everything looks correct 🤔

@Jakob3xD
Copy link
Collaborator

Jakob3xD commented Oct 7, 2025

The linter should hopefully work again once #740 got merged and you rebase it.

@Jakob3xD
Copy link
Collaborator

Jakob3xD commented Oct 9, 2025

My MR got merged. Please rebase and check if the issue still persists.

@ste93cry ste93cry force-pushed the support-reload-search-analyzers-api-endpoint branch from 5824c4c to 9d752b2 Compare October 10, 2025 12:14
@ste93cry
Copy link
Contributor Author

We're good to go, but there are a few flaky tests that need a second run 🚀

@ste93cry ste93cry requested a review from Jakob3xD October 10, 2025 12:20
@Jakob3xD Jakob3xD merged commit e31c031 into opensearch-project:main Oct 13, 2025
105 of 106 checks passed
@ste93cry ste93cry deleted the support-reload-search-analyzers-api-endpoint branch October 13, 2025 10:51
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.

[FEATURE] Expose the action to refresh the search analyzers

2 participants