-
Notifications
You must be signed in to change notification settings - Fork 58
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
[Feature] Send raw json request #177
[Feature] Send raw json request #177
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #177 +/- ##
============================================
+ Coverage 26.97% 27.03% +0.06%
- Complexity 2017 2018 +1
============================================
Files 263 263
Lines 7144 7150 +6
============================================
+ Hits 1927 1933 +6
Misses 5217 5217 ☔ View full report in Codecov by Sentry. |
7821edc
to
c07c566
Compare
dc76246
to
aafb793
Compare
Signed-off-by: imdhemy <imdhemy@gmail.com>
Signed-off-by: imdhemy <imdhemy@gmail.com>
Signed-off-by: imdhemy <imdhemy@gmail.com>
Signed-off-by: imdhemy <imdhemy@gmail.com>
Signed-off-by: imdhemy <imdhemy@gmail.com>
Signed-off-by: imdhemy <imdhemy@gmail.com>
798308f
to
7efd80b
Compare
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.
Very nice!
Do you think we should add .get
, .post
, etc, or maybe ._get
wrappers on top of this new method as well? Should those be the preferred ones recommended by the docs?
Thank you, @dblock! I went for I wasn't sure about the name, so I went for something similar to what's used in Guzzle. I wouldn't recommend an underscore prefix style, because it brings the bad old days of PHP to mind :). |
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.
Let's merge this.
Btw, link checker needs to be upgraded or locked |
Signed-off-by: imdhemy <imdhemy@gmail.com>
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.
Nice one :) thank you!
Thanks for your help @imdhemy! There's a million workitems in this client, would love some more help. Starting with guides/samples for example. |
@shyim @dblock My pleasure! :) I'd like to propose a slight change in the branches that could help improve this client.
Being the current version of OpenSearch, we need to set the |
We've been trying to decouple versioning in clients from OpenSearch proper. IMO the client should be broadly compatible with all versions of OpenSearch and follow its own semver. Does this change your recommendation? |
I'll think about you and may get back to you. Thank you |
Sounds good, I'll make time to add it. Thanks for opening #192! |
Description
This PR adds a wrapper
request()
method around the clientperformRequest()
to send Raw JSON requests.Issues Resolved
Closes #166
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.