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

refactor: replace payload with body for consistency with OpenSearch API #424

Merged
merged 14 commits into from
Dec 9, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- fix bug in `MLCommonClient_client.upload_model` by @rawwar in ([#336](https://github.com/opensearch-project/opensearch-py-ml/pull/336))
- fix lint issues on main by @rawwar in ([#374](https://github.com/opensearch-project/opensearch-py-ml/pull/374))
- fix CVE vulnerability by @rawwar in ([#383](https://github.com/opensearch-project/opensearch-py-ml/pull/383))
- refactor: replace 'payload' with 'body' in `create_standalone_connector` by @yerzhaisang ([#424](https://github.com/opensearch-project/opensearch-py-ml/pull/424))

## [1.1.0]

Expand Down
33 changes: 29 additions & 4 deletions opensearch_py_ml/ml_commons/model_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
# Any modifications Copyright OpenSearch Contributors. See
# GitHub history for details.

import warnings

from opensearchpy import OpenSearch

from opensearch_py_ml.ml_commons.ml_common_utils import ML_BASE_URI
Expand All @@ -14,12 +16,35 @@ class Connector:
def __init__(self, os_client: OpenSearch):
self.client = os_client

def create_standalone_connector(self, payload: dict):
if not isinstance(payload, dict):
raise ValueError("payload needs to be a dictionary")
def create_standalone_connector(self, body: dict = None, payload: dict = None):
Yerzhaisang marked this conversation as resolved.
Show resolved Hide resolved
"""
Create a standalone connector.

Parameters:
body (dict): The request body, preferred parameter.
payload (dict): Deprecated. Use 'body' instead.

Raises:
ValueError: If neither 'body' nor 'payload' is provided or if the provided argument is not a dictionary.
"""
if body is None and payload is None:
raise ValueError("A 'body' parameter must be provided as a dictionary.")

if payload is not None:
if not isinstance(payload, dict):
raise ValueError("'payload' needs to be a dictionary.")
warnings.warn(
"'payload' is deprecated. Please use 'body' instead.",
DeprecationWarning,
)
# Use `payload` if `body` is not provided for backward compatibility
body = body or payload

if not isinstance(body, dict):
raise ValueError("'body' needs to be a dictionary.")

return self.client.transport.perform_request(
method="POST", url=f"{ML_BASE_URI}/connectors/_create", body=payload
method="POST", url=f"{ML_BASE_URI}/connectors/_create", body=body
)

def list_connectors(self):
Expand Down
19 changes: 14 additions & 5 deletions tests/ml_commons/test_model_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _safe_delete_connector(client, connector_id):

Choose a reason for hiding this comment

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

can you add a test when payload is passed as an argument? also when it does not conform to the datatype

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pyek-bot could you please check added unit test?


@pytest.fixture
def connector_payload():
def connector_body():
return {
"name": "Test Connector",
"description": "Connector for testing",
Expand All @@ -52,8 +52,8 @@ def connector_payload():


@pytest.fixture
def test_connector(client: Connector, connector_payload: dict):
res = client.create_standalone_connector(connector_payload)
def test_connector(client: Connector, connector_body: dict):
res = client.create_standalone_connector(connector_body)
connector_id = res["connector_id"]
yield connector_id

Expand All @@ -64,8 +64,8 @@ def test_connector(client: Connector, connector_payload: dict):
OPENSEARCH_VERSION < CONNECTOR_MIN_VERSION,
reason="Connectors are supported in OpenSearch 2.9.0 and above",
)
def test_create_standalone_connector(client: Connector, connector_payload: dict):
res = client.create_standalone_connector(connector_payload)
def test_create_standalone_connector(client: Connector, connector_body: dict):
res = client.create_standalone_connector(connector_body)
assert "connector_id" in res

_safe_delete_connector(client, res["connector_id"])
Expand All @@ -74,6 +74,15 @@ def test_create_standalone_connector(client: Connector, connector_payload: dict)
client.create_standalone_connector("")


@pytest.mark.parametrize("invalid_body", ["", None, [], 123])
def test_create_standalone_connector_invalid_body(client: Connector, invalid_body):
with pytest.raises(
ValueError,
match=r"A 'body' parameter must be provided as a dictionary|'body' needs to be a dictionary",
):
client.create_standalone_connector(body=invalid_body)


@pytest.mark.skipif(
OPENSEARCH_VERSION < CONNECTOR_MIN_VERSION,
reason="Connectors are supported in OpenSearch 2.9.0 and above",
Expand Down
Loading