opensearch-project / opensearch-py

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

Fix: re-add service to fix integration with LangChain. #603

Closed dblock closed 10 months ago

dblock commented 10 months ago

Description

Re-add .service, which is used by LangChain to figure out whether we're talking to AOSS. This broke in https://github.com/opensearch-project/opensearch-py/pull/547.

Issues Resolved

Closes https://github.com/opensearch-project/opensearch-py/issues/600

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.

navneet1v commented 10 months ago

@dblock its not necessary related to AOSS. Even in AOS/OSS, if user is asking for OpenSearch service to generate the ids for the data, os client should not send _id in the bulk request.

dblock commented 10 months ago

@dblock its not necessary related to AOSS. Even in AOS/OSS, if user is asking for OpenSearch service to generate the ids for the data, os client should not send _id in the bulk request.

The code is pretty explicit :)

def _is_aoss_enabled(http_auth: Any) -> bool:
    """Check if the service is http_auth is set as `aoss`."""
    if (
        http_auth is not None
        and hasattr(http_auth, "service")
        and http_auth.service == "aoss"
    ):
        return True
    return False
navneet1v commented 10 months ago

@dblock its not necessary related to AOSS. Even in AOS/OSS, if user is asking for OpenSearch service to generate the ids for the data, os client should not send _id in the bulk request.

The code is pretty explicit :)

def _is_aoss_enabled(http_auth: Any) -> bool:
    """Check if the service is http_auth is set as `aoss`."""
    if (
        http_auth is not None
        and hasattr(http_auth, "service")
        and http_auth.service == "aoss"
    ):
        return True
    return False

This code is explicit, because AOSS doesn't support _id. But given that client is generic and AOSS support _id for search collection. So if we make this change then search collections will break.

@dblock

This is the reason why I am saying the expectation from opensearch-py is if user has not provided _id, client should not add of its own whether it is AOS, OSS or AOSS.

dblock commented 10 months ago

This is the reason why I am saying the expectation from opensearch-py is if user has not provided _id, client should not add of its own whether it is AOS, OSS or AOSS.

Agreed. It does not. Am I missing anything?