opensearch-project / opensearch-py

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

[Doc] Improve opensearch.client documentation #718

Open Plutone11011 opened 3 months ago

Plutone11011 commented 3 months ago

Related to #380

Opensearch client methods create, index, deletecould benefit from:

I can contribute to this issue

rbpasker commented 3 weeks ago

+1 on the 'error handling'

it seems that HTTP responses (eg GET /<index>/_search AKA response = client.search(index="<index>") on a non-existing index) are thrown as errors rather than returned in the response. This is unexpected since response is the nominal return variable for python requests, where one would check response.status_code == 404.

This behavior should be clearly explained in the user guide

https://opensearch-project.github.io/opensearch-py/api-ref/exceptions.html

dblock commented 3 weeks ago

We could use a detailed error handling guide in https://github.com/opensearch-project/opensearch-py/tree/main/guides. Ultimately we want all API docs to be generated from https://github.com/opensearch-project/opensearch-api-specification, and all client-language-specific things to be in the GitHub repo. The API-ref that's auto-published to https://opensearch-project.github.io/opensearch-py/ could have the guides published in a nicer format, too.

Your help is much appreciated!

rbpasker commented 3 weeks ago

its actually worse because there's no semantic error checking, eg

*** Error: RequestError(400, 'resource_already_exists_exception', 'index [opinions/FduukSQQQcuRcp4ZK0w51g] already exists') requires:

except TransportError as e:
    if e.status_code == 400 and e.error == 'resource_already_exists_exception':
        # Handle the specific exception
        print("Resource already exists.")
    else:
        # Handle other TransportError exceptions
        print(f"TransportError occurred: {e}")

so i have to write a helper that converts exceptions to semantic errors:

except TransportError as e:
     return transform_exception_to_return_value(e)

https://opensearch-project.github.io/opensearch-py/api-ref/exceptions.html#opensearchpy.TransportError

dblock commented 3 weeks ago

You're saying that resource_already_exists_exception should be a specialized ResourceAlreadyExistsError? That would make sense, it's a feature request (please open?).

rbpasker commented 3 weeks ago

yep, there are probably also other errors that I haven't encountered yet that could probably benefit from specialization