-
Notifications
You must be signed in to change notification settings - Fork 186
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
Add override for code generator to change indices.put_alias
argument order
#804
Add override for code generator to change indices.put_alias
argument order
#804
Conversation
Signed-off-by: Jeppe Fihl-Pearson <jeppe@memrise.com>
Signed-off-by: Jeppe Fihl-Pearson <jeppe@memrise.com>
Signed-off-by: Jeppe Fihl-Pearson <jeppe@memrise.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me! Thanks for fixing this.
@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. |
You're welcome! No, I haven't looked through changes to other APIs. I only noticed the issue with |
I looked at the 2.6.0 - 2.7.0 diff and found one more change in cc: @DarshitChanpura |
@dblock this is interesting, post is exposed but doesn't really consume any body for the request. Also, could you point the exact line change for post_dashboards_info that might be causing this. v2.6.0...v2.7.0#diff-d373e8c1b532ced908538444607c89e5a80cfe7a24ef5c81a3708e0fb2cf48fdL2173 doesn't seem to match any text when searching for "dashboards_info". Maybe I missed something? |
@DarshitChanpura Thanks. I think we're all good, since this method doesn't take a 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. |
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.