opensearch-py icon indicating copy to clipboard operation
opensearch-py copied to clipboard

[Doc] Improve opensearch.client documentation

Open Plutone11011 opened this issue 10 months ago • 5 comments

Related to #380

Opensearch client methods create, index, delete could benefit from:

  • updated parameters description in documentation, as well as return value and error handling, if relevant
  • return values and error handling in user guide

I can contribute to this issue

Plutone11011 avatar Apr 13 '24 20:04 Plutone11011

+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

rbpasker avatar Jun 25 '24 12:06 rbpasker

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!

dblock avatar Jun 25 '24 15:06 dblock

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

rbpasker avatar Jun 25 '24 16:06 rbpasker

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

dblock avatar Jun 25 '24 16:06 dblock

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

rbpasker avatar Jun 25 '24 16:06 rbpasker