Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Added client-level REST helpers. #544

Merged
merged 5 commits into from
Nov 13, 2023
Merged

Added client-level REST helpers. #544

merged 5 commits into from
Nov 13, 2023

Conversation

dblock
Copy link
Member

@dblock dblock commented Oct 17, 2023 •
edited
Loading

Description

Adds client-level REST helpers.

This allows one to do client.http.get(...), .post, etc., instead of client.transport.perform_request("GET", ...).

Issues Resolved

Closes #498.

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
Copy link

codecov bot commented Oct 17, 2023 •
edited
Loading

Codecov Report

Merging #544 (06a59dd) into main (f02d6de) will increase coverage by 0.12%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #544      +/-   ##
==========================================
+ Coverage   71.74%   71.87%   +0.12%     
==========================================
  Files          87       89       +2     
  Lines        7886     7922      +36     
==========================================
+ Hits         5658     5694      +36     
  Misses       2228     2228
Files Coverage Δ
opensearchpy/_async/client/__init__.py 46.18% <100.00%> (+0.39%) ⬆️
opensearchpy/_async/client/http.py 100.00% <100.00%> (ø)
opensearchpy/client/__init__.py 61.45% <100.00%> (+0.28%) ⬆️
opensearchpy/client/http.py 100.00% <100.00%> (ø)

@dblock dblock force-pushed the rest-helpers branch 2 times, most recently from 4d20438 to aec5726 Compare October 17, 2023 21:14
@dblock
Copy link
Member Author

dblock commented Oct 17, 2023

@saimedhi @harshavamsi @Xtansia wdyt?

@dblock dblock force-pushed the rest-helpers branch 4 times, most recently from 1f9b9f1 to 6fdce84 Compare October 17, 2023 21:34
@Xtansia
Copy link
Contributor

Xtansia commented Oct 17, 2023

@saimedhi @harshavamsi @Xtansia wdyt?

We're going to have this name clash with get in all the clients presumably, and it'd probably be nice to have this semi-standardised across them. An option is to make an "operation namespace" like raw or http same as we have cat/cluster/indices etc.

@dblock
Copy link
Member Author

dblock commented Oct 18, 2023

@Xtansia What's your most preferred option for client.X.Y.Z?

@Xtansia
Copy link
Contributor

Xtansia commented Oct 23, 2023

@Xtansia What's your most preferred option for client.X.Y.Z?

I personally feel like client.http.get() is probably the most self-documenting, and least likely to ever conflict with the naming of a "real" operation

@dblock dblock marked this pull request as ready for review November 9, 2023 21:27
@dblock
Copy link
Member Author

dblock commented Nov 9, 2023

Followed suggestion from @Xtansia to make the code client.http.get, client.http.post, etc. This is ready to review!

@dblock dblock force-pushed the rest-helpers branch 2 times, most recently from 3ee1b76 to 970a5fa Compare November 10, 2023 13:59
@dblock dblock mentioned this pull request Nov 10, 2023
Closed
VachaShah
VachaShah reviewed Nov 12, 2023
View reviewed changes
Copy link
Collaborator

@VachaShah VachaShah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just a few minor comments.

guides/json.md Outdated
@@ -6,61 +6,75 @@

# Making Raw JSON REST Requests

The OpenSearch client implements many high-level REST DSLs that invoke OpenSearch APIs. However you may find yourself in a situation that requires you to invoke an API that is not supported by the client. Use `client.transport.perform_request` to do so. See [samples/json](../samples/json) for a complete working sample.
The OpenSearch client implements many high-level REST DSLs that invoke OpenSearch APIs. However you may find yourself in a situation that requires you to invoke an API that is not supported by the client. Use `client.http.get`, `head` , `put`, `post`, and `delete` in older versions to do so. See [samples/json](../samples/json) for a complete working sample.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use client.transport.perform_request in older versions (<= 2.3.x) and client.http.get and others in newer versions?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think "in older versions" would be removed from this line: "Use client.http.get, head , put, post, and delete to do so"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eyes, fixed.

test_opensearchpy/test_async/test_client.py
# Modifications Copyright OpenSearch Contributors. See
# GitHub history for details.
#
# Licensed to Elasticsearch B.V. under one or more contributor
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this header needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because the file was copied.

@dblock
Added client-level REST helpers.
f42c641 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Move functions into an .http namespace.
910787f 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Poetry update in samples.
c1766b5 
Signed-off-by: dblock <dblock@amazon.com>
@dblock
Fix: typo.
8af51f2 
Signed-off-by: dblock <dblock@amazon.com>
@dblock dblock mentioned this pull request Nov 13, 2023
Merged
@dblock
Clarified what to use in which older versions.
06a59dd 
Signed-off-by: dblock <dblock@amazon.com>
@VachaShah VachaShah merged commit e68b9e7 into opensearch-project:main Nov 13, 2023
54 checks passed
roma2023 pushed a commit to roma2023/opensearch-py that referenced this pull request Dec 28, 2023
@dblock @roma2023
Added client-level REST helpers. (opensearch-project#544)
6aaa52c 
* Added client-level REST helpers.

Signed-off-by: dblock <dblock@amazon.com>

* Move functions into an .http namespace.

Signed-off-by: dblock <dblock@amazon.com>

* Poetry update in samples.

Signed-off-by: dblock <dblock@amazon.com>

* Fix: typo.

Signed-off-by: dblock <dblock@amazon.com>

* Clarified what to use in which older versions.

Signed-off-by: dblock <dblock@amazon.com>

---------

Signed-off-by: dblock <dblock@amazon.com>
Signed-off-by: roma2023 <romasaparhan19@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VachaShah VachaShah VachaShah approved these changes

@harshavamsi harshavamsi Awaiting requested review from harshavamsi harshavamsi is a code owner

@axeoman axeoman Awaiting requested review from axeoman axeoman is a code owner

@deztructor deztructor Awaiting requested review from deztructor

@Shephalimittal Shephalimittal Awaiting requested review from Shephalimittal Shephalimittal is a code owner

@saimedhi saimedhi Awaiting requested review from saimedhi saimedhi is a code owner

@florianvazelle florianvazelle Awaiting requested review from florianvazelle florianvazelle is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add the ability to send REST requests via the client
3 participants
@dblock @Xtansia @VachaShah