opensearch-project
Add target_index_settings to rollup specification
Description
Allow to create index with specified settings during rollup
Issues Resolved
https://github.com/opensearch-project/index-management/issues/1376
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.
@MrChaos1993 amend your commit with -s to pass DCO
Exercise an API in a test that returns this data, see https://github.com/opensearch-project/opensearch-api-specification/blob/main/TESTING_GUIDE.md
Changes Analysis
Commit SHA: f9d87fd88d175abefc8b17aa36e1484aee7ed480 Comparing To SHA: a8ea65db455a2752f250df5694fa1e2195b57b87
API Changes
Summary
└─┬Components
└─┬rollups._common___Rollup
└──[➕] properties (65465:9)
| Document Element | Total Changes | Breaking Changes |
|---|---|---|
| components | 1 | 0 |
- Total Changes: 1
- Additions: 1
Report
The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/15781582685/artifacts/3375164786
API Coverage
| Before | After | Δ | |
|---|---|---|---|
| Covered (%) | 666 (65.23 %) | 666 (65.23 %) | 0 (0 %) |
| Uncovered (%) | 355 (34.77 %) | 355 (34.77 %) | 0 (0 %) |
| Unknown | 91 | 91 | 0 |
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Test suites that involve the new test failed with Expected status 201, but received 400: application/json. Invalid field [target_index_settings] found in Rollup.
Don't forget to add a line in the CHANGELOG.md describing the change from this PR.
We still get this error Invalid field [target_index_settings] found in Rollup. in the test suite against OS 3.0.0
Was target_index_settings added to 3.0.0? Or was it added to a later/unreleased version?
It was merged to main branch (Link to PR) and require version 3.0.0 or above. I am not sure is it released or not, could you please help me?
@MrChaos1993 That looks like it belongs to an unreleased version of the index-management plugin.
@dblock correct me if I'm wrong but we only have the capability to test against unreleased OpenSearch right now. And It's too complicated to setup a cluster with unreleased plugins. So, in that case, we should this PR on hold for now?
@dblock correct me if I'm wrong but we only have the capability to test against unreleased OpenSearch right now. And It's too complicated to setup a cluster with unreleased plugins. So, in that case, we should this PR on hold for now?
You could use an unreleased version of the plugin too by installing the specific build in the container, but unsure it's worth it.
@dblock correct me if I'm wrong but we only have the capability to test against unreleased OpenSearch right now. And It's too complicated to setup a cluster with unreleased plugins. So, in that case, we should this PR on hold for now?
You could use an unreleased version of the plugin too by installing the specific build in the container, but unsure it's worth it.
I don't think it's worth it to set it up per PR like this, just to remove it once the new version of the plugin is released. Let's wait, @MrChaos1993. Sorry we don't have a more elegant solution for this right now. I've created a new label for this type of PRs.
@dblock correct me if I'm wrong but we only have the capability to test against unreleased OpenSearch right now. And It's too complicated to setup a cluster with unreleased plugins. So, in that case, we should this PR on hold for now?
You could use an unreleased version of the plugin too by installing the specific build in the container, but unsure it's worth it.
I don't think it's worth it to set it up per PR like this, just to remove it once the new version of the plugin is released. Let's wait, @MrChaos1993. Sorry we don't have a more elegant solution for this right now. I've created a new label for this type of PRs.
Sure, no problem! I just wondering about missing specification for the new field, so If it's better to wait until release a newer version of plugin I totally agree. Only one question should I tag someone when a newer version of the plugin will be released, or you will merge\restart check on your own?
Sure, no problem! I just wondering about missing specification for the new field, so If it's better to wait until release a newer version of plugin I totally agree. Only one question should I tag someone when a newer version of the plugin will be released, or you will merge\restart check on your own?
We'll need to update this PR to point the test with the new params to the release that can accept that params. We will check back in a couple of month. But if you notice that it's been released sooner than that, feel free to leave a message here :) Thank you so much for helping us with updating this endpoint.
@nhtruong, Hi! I see that version 3.0.0 has been released and this pull request can be merged :)
As I see, tests are failing due to reasons unrelated to this PR. Please check the build logs for more details.
@MrChaos1993 We'll need some help.
- Rebase, these issues may already be fixed.
- If it fails again, look at the errors, if the error is not related to this PR make sure there's an issue about it, if not open one - for example if a test is failing in this PR but succeeds on main, open something that says "XYZ test is flaky".
- Comment on this issue with all the unrelated failure issues.
- I can kick off a retry (doing this now for the failures here), or you can force it with a rebase.