opensearch-project / opensearch-py

Python Client for OpenSearch
https://opensearch.org/docs/latest/clients/python/
Apache License 2.0
338 stars 170 forks source link

Add override for code generator to change `indices.put_alias` argument order #804

Closed Tenzer closed 1 month ago

Tenzer commented 1 month ago

Description

This solves an issue introduced in v2.7.0 where the argument order of the indices.put_alias() method were altered, resulting in a breaking change for any user that called the function without naming the arguments provided.

Issues Resolved

Closes #803.

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.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.32%. Comparing base (ba715b9) to head (4be23a6). Report is 54 commits behind head on main.

Files Patch % Lines
opensearchpy/_async/client/indices.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #804 +/- ## ========================================== - Coverage 71.95% 70.32% -1.64% ========================================== Files 91 113 +22 Lines 8001 8868 +867 ========================================== + Hits 5757 6236 +479 - Misses 2244 2632 +388 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dblock commented 1 month ago

@Tenzer did you happen to go through a diff of other APIs that might have this problem? Maybe there's a way to do this systematically and add tests, including in the generator 🤔 I want to double check before cutting a newer release.

Tenzer commented 1 month ago

You're welcome! No, I haven't looked through changes to other APIs. I only noticed the issue with put_alias() because we used that as part of our test code.

dblock commented 1 month ago

I looked at the 2.6.0 - 2.7.0 diff and found one more change in post_dashboards_info, https://github.com/opensearch-project/opensearch-py/compare/v2.6.0...v2.7.0#diff-d373e8c1b532ced908538444607c89e5a80cfe7a24ef5c81a3708e0fb2cf48fdL2173, I think it's cause it's missing a request body in the spec.

cc: @DarshitChanpura

DarshitChanpura commented 1 month ago

@dblock this is interesting, post is exposed but doesn't really consume any body for the request. https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/rest/DashboardsInfoAction.java#L94 We only seem to be using get request: https://github.com/opensearch-project/security-dashboards-plugin/blob/main/server/routes/index.ts#L584 https://github.com/opensearch-project/security-dashboards-plugin/blob/main/public/utils/dashboards-info-utils.tsx#L21-L34

Also, could you point the exact line change for post_dashboards_info that might be causing this. https://github.com/opensearch-project/opensearch-py/compare/v2.6.0...v2.7.0#diff-d373e8c1b532ced908538444607c89e5a80cfe7a24ef5c81a3708e0fb2cf48fdL2173 doesn't seem to match any text when searching for "dashboards_info". Maybe I missed something?

dblock commented 1 month ago

@DarshitChanpura Thanks. I think we're all good, since this method doesn't take a body it was a bug fix.

I am sure where this is evaluated in the client code generator, but you can dig through https://github.com/opensearch-project/opensearch-py/blob/main/utils/generate_api.py.