Closed derek-ho closed 8 months ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 79.71%. Comparing base (
360e88c
) to head (4240ad3
). Report is 11 commits behind head on main.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
https://github.com/opensearch-project/opensearch-rs/blob/main/.github/workflows/test.yml#L36 - if this becomes 2.12.0 or onwards, then this change is necessary. For now this is unnecessary. Should the tests be run with a matrix of different opensearch versions to ensure compatibility?
https://github.com/opensearch-project/opensearch-rs/blob/main/.github/workflows/test.yml#L36 - if this becomes 2.12.0 or onwards, then this change is necessary. For now this is unnecessary. Should the tests be run with a matrix of different opensearch versions to ensure compatibility?
The linked line is only used for a handful of very basic tests, the real integration tests where we use core's YAML tests is this matrix here: https://github.com/opensearch-project/opensearch-rs/blob/main/.github/workflows/test.yml#L66
Got it. @Xtansia this change is starting in 2.12.0 onwards. I am seeing in the matrix this wouldn't be tested. Can you suggest how you think I can change this PR to best pickup the changes from 2.12.0 onwards?
@Xtansia @dblock can you confirm what is the plan to add 2.12.0 when released into that matrix, so I can modify this PR to add support for the new admin credential changes? If there is no plan then I think we can close this PR and mark this repo as compliant with the new credential changes, thanks!
@derek-ho AFAIK when 2.12 is available we'll add it to CI. I think you can mark this as draft as other PRs?
Now that 2.12 is released this should be unblocked. Would the maintainers review and merge this?
@derek-ho Could you please add 2.12.0
to the various workflow matrices so that we can confirm things are working as expected?
@Xtansia thanks for the suggestion. I used your logic to extract the version, but used the same logic as opensearch-build: https://github.com/opensearch-project/opensearch-build/blob/main/scripts/default/integtest.sh to determine whether it is >= 2.12, just to be consistent. It looks like tests are passing and PR is in a good state. Please merge when it looks good, thanks!
Thanks @derek-ho!
Description
Fixes the CI to pass in "admin" as the admin password if the version is >= 2.12. This is necessary since 2.12 and onwards requires an initial admin password to be set in order to run the security demo configuration.
Issues Resolved
List any issues this PR will resolve, e.g. Closes [...].
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.