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.

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

Updated APIs to match other language clients and opensearch openapi spec #502

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

saimedhi
Copy link
Collaborator

@saimedhi saimedhi commented Sep 16, 2023

Description

Updated APIs to match other language clients and opensearch openapi spec. APIs added to "_patch" are manually written. Other APIs added are Generated using openapispec.

Deprecated APIs: They are moved into client/_patch.py
list_all_point_in_time, create_point_in_time, health_check, update_audit_config, delete_point_in_time

Newly Added APIs: They are moved into corresponding namespaces
get_all_pits, create_pit, health, update_audit_configuration, delete_all_pits, delete_pit

Issues Resolved

Closes #499, #500, #501

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 Sep 16, 2023

Codecov Report

Merging #502 (77cd26a) into main (c8b04a5) will decrease coverage by 0.06%.
Report is 2 commits behind head on main.
The diff coverage is 70.73%.

@@            Coverage Diff             @@
##             main     #502      +/-   ##
==========================================
- Coverage   70.92%   70.86%   -0.06%     
==========================================
  Files          81       83       +2     
  Lines        7732     7796      +64     
==========================================
+ Hits         5484     5525      +41     
- Misses       2248     2271      +23     
Files Changed Coverage Δ
opensearchpy/_async/client/_patch.py 46.15% <46.15%> (ø)
opensearchpy/_async/client/__init__.py 44.24% <72.72%> (+0.28%) ⬆️
opensearchpy/client/_patch.py 76.92% <76.92%> (ø)
opensearchpy/client/__init__.py 60.79% <90.90%> (+0.35%) ⬆️
opensearchpy/_async/client/security.py 43.98% <100.00%> (+0.26%) ⬆️
opensearchpy/client/security.py 43.98% <100.00%> (+0.26%) ⬆️

@saimedhi saimedhi force-pushed the merge-gen-client-final branch 2 times, most recently from a99619c to 3fc5b16 Compare September 16, 2023 01:15


@query_params()
async def list_all_point_in_time(self, params=None, headers=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purpose of moving these APIs to patch file? Is the patch file going to be a place for all manual APIs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

@@ -1955,64 +1961,66 @@ async def get_script_languages(self, params=None, headers=None):
"GET", "/_script_language", params=params, headers=headers
)

@query_params()
async def list_all_point_in_time(self, params=None, headers=None):
async def create_pit(self, index, params=None, headers=None):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these APIs generated or manually written?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

generated

opensearchpy/_async/client/__init__.py Show resolved Hide resolved
@saimedhi saimedhi force-pushed the merge-gen-client-final branch from 3fc5b16 to 20fb3f2 Compare September 19, 2023 21:23
@saimedhi saimedhi marked this pull request as draft September 19, 2023 22:34
@saimedhi saimedhi force-pushed the merge-gen-client-final branch from 20fb3f2 to 77cd26a Compare September 19, 2023 22:39
@saimedhi saimedhi marked this pull request as ready for review September 19, 2023 22:48
@saimedhi saimedhi requested a review from VachaShah September 19, 2023 22:48
@saimedhi
Copy link
Collaborator Author

Hi @florianvazelle, I would greatly appreciate it if you could kindly review this PR and share your feedback whenever it's convenient for you. Thank you!

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.

This looks great! Can we also start adding the generation details in a markdown file? For example, what the patch file is supposed to be? How the generator works etc? I see there is already https://github.com/opensearch-project/opensearch-py/blob/main/DEVELOPER_GUIDE.md#running-python-client-generator

@saimedhi
Copy link
Collaborator Author

This looks great! Can we also start adding the generation details in a markdown file? For example, what the patch file is supposed to be? How the generator works etc? I see there is already https://github.com/opensearch-project/opensearch-py/blob/main/DEVELOPER_GUIDE.md#running-python-client-generator

@VachaShah, I'll include these details in my upcoming PR. I'll create a markdown file explaining the generator's functionality and the client structure, including what's placed in patch files. You can find most of the information here, and I'll ensure it's added to the repository soon.

@saimedhi
Copy link
Collaborator Author

@VachaShah, Shall we merge this

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Looks good.

For generated files we need some kind of "this is generated" header.

@dblock dblock merged commit c6c7df5 into opensearch-project:main Sep 26, 2023
@saimedhi saimedhi deleted the merge-gen-client-final branch October 4, 2023 22:19
roma2023 pushed a commit to roma2023/opensearch-py that referenced this pull request Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

APIs with Naming differences (Generated vs Existing Python Client)
3 participants