Implement model, model group and connector apis #170

acha5066 · 2023-11-23T04:47:42Z

Description

This PR implements the model, model group and connector apis from the ML Commons API so that developers can set up connectors to 3rd party tools like OpenAI as described here: https://github.com/opensearch-project/ml-commons/blob/main/docs/remote_inference_blueprints/openai_connector_embedding_blueprint.md

Issues Resolved

Closes #169

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.

dblock

A cursory look at it, looks ok. It needs tests and documentation, please. I would write a working sample as well. Sign commits with -s to pass DCO.

I encourage anyone wanting to add APIs to spend time doing #97 instread of hand-rolling these.

acha5066 · 2023-12-05T03:49:35Z

Added tests for the new namespace methods.
Added sample usage to USER_GUIDE.md.
Added documentation to the namespace class header. Linked to relevant API docs. Wasn't sure what documentation you had in mind so please let me know.

Regarding DCO I did signoff with Andrew Chappell and changed my username like: git config --global user.name "Andrew Chappell" so not sure why that isn't reflected. It keeps associating with andrew.chappell 🤔

codecov · 2023-12-05T20:25:06Z

Codecov Report

Attention: 84 lines in your changes are missing coverage. Please review.

Comparison is base (9ebb183) 24.19% compared to head (e55cd04) 25.99%.

❗ Current head e55cd04 differs from pull request most recent head 8a516e5. Consider uploading reports for the commit 8a516e5 to get more accurate results

Files	Patch %	Lines
...OpenSearch/Namespaces/MachineLearningNamespace.php	85.81%	20 Missing ⚠️
...nts/MachineLearning/Connectors/DeleteConnector.php	50.00%	5 Missing ⚠️
...points/MachineLearning/Connectors/GetConnector.php	50.00%	5 Missing ⚠️
...s/MachineLearning/ModelGroups/DeleteModelGroup.php	50.00%	5 Missing ⚠️
...s/MachineLearning/ModelGroups/UpdateModelGroup.php	50.00%	5 Missing ⚠️
...h/Endpoints/MachineLearning/Models/DeleteModel.php	50.00%	5 Missing ⚠️
...h/Endpoints/MachineLearning/Models/DeployModel.php	50.00%	5 Missing ⚠️
...arch/Endpoints/MachineLearning/Models/GetModel.php	50.00%	5 Missing ⚠️
...earch/Endpoints/MachineLearning/Models/Predict.php	50.00%	5 Missing ⚠️
...Endpoints/MachineLearning/Models/UndeployModel.php	50.00%	5 Missing ⚠️
... and 8 more

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #170      +/-   ##
============================================
+ Coverage     24.19%   25.99%   +1.79%     
- Complexity     1938     2018      +80     
============================================
  Files           246      263      +17     
  Lines          6865     7145     +280     
============================================
+ Hits           1661     1857     +196     
- Misses         5204     5288      +84

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dblock

I made a bunch of nitpicky comments. @shyim should take a look at the code.

For DCO I think you will have to amend previous commits. I would make a new branch off main and merge the code and re-commit it and force push the branch. LMK if you need help on how to do that!

dblock · 2023-12-05T20:25:25Z

CHANGELOG.md

 - Added support for `format` parameter to specify the sql response format ([#161](https://github.com/opensearch-project/opensearch-php/pull/161))
+- Implemented the Model, Model Group and Connector apis ([#170](https://github.com/opensearch-project/opensearch-php/pull/170))


Let's say Added ml-commons model, model group, and connector APIs to be consistent with the rest?

dblock · 2023-12-05T20:25:46Z

USER_GUIDE.md

@@ -24,7 +24,7 @@ class MyOpenSearchClass

    public function __construct()
    {
-        //simple Setup 
+        //simple Setup


// space (I know it was there before, but let's fix - generally I find these comments sort of useless, but we should at least Capitalize and Space Them Consistently :)

dblock · 2023-12-05T20:27:13Z

USER_GUIDE.md

@@ -501,3 +501,69 @@ $client = \OpenSearch\ClientBuilder::fromConfig($config);

 ...
 ```
+## Machine Learning Example Usage


Let's extract this into its own user guide like in https://github.com/opensearch-project/opensearch-py/tree/main/guides? It will be easier to grok. Link the guides from the user guide under maybe a new advanced or other topics section?

dblock · 2023-12-05T20:27:48Z

USER_GUIDE.md

@@ -501,3 +501,69 @@ $client = \OpenSearch\ClientBuilder::fromConfig($config);

 ...
 ```
+## Machine Learning Example Usage
+
+This example assumes you are using the AWS managed OpenSearch


No need to wrap in markdown

dblock · 2023-12-05T20:28:20Z

USER_GUIDE.md

+## Machine Learning Example Usage
+
+This example assumes you are using the AWS managed OpenSearch
+service. See [The ML Commons documentation for more examples and further information.](https://github.com/opensearch-project/ml-commons/blob/main/docs/remote_inference_blueprints/openai_connector_embedding_blueprint.md)


Capitalize ml-commons consistently, it's lowercase and just link that word, so [ml commons documentation](...).

dblock · 2023-12-05T20:29:12Z

USER_GUIDE.md

+service. See [The ML Commons documentation for more examples and further information.](https://github.com/opensearch-project/ml-commons/blob/main/docs/remote_inference_blueprints/openai_connector_embedding_blueprint.md)
+
+It walks through the process of setting up a model to generate
+vector embeddings from OpenAI.


We should probably add a prerequisites section of where to sign up / get keys from OpenAI.

Signed-off-by: Andrew Chappell <[email protected]>

acha5066 · 2023-12-13T23:51:43Z

DCO should be fine now

dblock · 2023-12-15T15:38:11Z

@shyim Got some time to CR this?

shyim

Looks good

acha5066 requested review from VachaShah, dblock, harshavamsi and shyim as code owners November 23, 2023 04:47

dblock reviewed Nov 24, 2023

View reviewed changes

dblock reviewed Dec 5, 2023

View reviewed changes

acha5066 added 12 commits December 14, 2023 10:47

Add createConnector to the machine learning namespace

d13f76d

Signed-off-by: Andrew Chappell <[email protected]>

Implement connectors API

9ee5ae8

Signed-off-by: Andrew Chappell <[email protected]>

Added model group apis

76eb0c3

Signed-off-by: Andrew Chappell <[email protected]>

Implement Models API

49dd58d

Signed-off-by: Andrew Chappell <[email protected]>

Update license header for new files only

33a8ce7

Signed-off-by: Andrew Chappell <[email protected]>

Update changelog

8b1ad46

Signed-off-by: Andrew Chappell <[email protected]>

Added ability to search all models

b587a34

Signed-off-by: Andrew Chappell <[email protected]>

Add Tasks namespace

4ba4da1

Signed-off-by: Andrew Chappell <[email protected]>

Added tests for machine learning namespace and ran cs fixer

8cf6f87

Signed-off-by: Andrew Chappell <[email protected]>

Added sample usage to documentation

7ec9027

Signed-off-by: Andrew Chappell <[email protected]>

Added documentation to namespace header

a47b798

Signed-off-by: Andrew Chappell <[email protected]>

Updates based on feedback

8a516e5

Signed-off-by: Andrew Chappell <[email protected]>

acha5066 force-pushed the new-apis branch from ee83900 to 8a516e5 Compare December 13, 2023 23:50

shyim approved these changes Dec 16, 2023

View reviewed changes

dblock approved these changes Dec 18, 2023

View reviewed changes

dblock merged commit c1fc911 into opensearch-project:main Dec 18, 2023
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement model, model group and connector apis #170

Implement model, model group and connector apis #170

acha5066 commented Nov 23, 2023

dblock left a comment •

edited

Loading

acha5066 commented Dec 5, 2023

codecov bot commented Dec 5, 2023 •

edited

Loading

dblock left a comment

dblock Dec 5, 2023

dblock Dec 5, 2023

dblock Dec 5, 2023

dblock Dec 5, 2023

dblock Dec 5, 2023

dblock Dec 5, 2023

acha5066 commented Dec 13, 2023

dblock commented Dec 15, 2023

shyim left a comment

		- Added support for `format` parameter to specify the sql response format ([#161](https://github.com/opensearch-project/opensearch-php/pull/161))
		- Implemented the Model, Model Group and Connector apis ([#170](https://github.com/opensearch-project/opensearch-php/pull/170))

Implement model, model group and connector apis #170

Implement model, model group and connector apis #170

Conversation

acha5066 commented Nov 23, 2023

Description

Issues Resolved

dblock left a comment • edited Loading

Choose a reason for hiding this comment

acha5066 commented Dec 5, 2023

codecov bot commented Dec 5, 2023 • edited Loading

Codecov Report

dblock left a comment

Choose a reason for hiding this comment

dblock Dec 5, 2023

Choose a reason for hiding this comment

dblock Dec 5, 2023

Choose a reason for hiding this comment

dblock Dec 5, 2023

Choose a reason for hiding this comment

dblock Dec 5, 2023

Choose a reason for hiding this comment

dblock Dec 5, 2023

Choose a reason for hiding this comment

dblock Dec 5, 2023

Choose a reason for hiding this comment

acha5066 commented Dec 13, 2023

dblock commented Dec 15, 2023

shyim left a comment

Choose a reason for hiding this comment

dblock left a comment •

edited

Loading

codecov bot commented Dec 5, 2023 •

edited

Loading