Rename APIs for model serving framework #159

AlibiZhenis · 2023-05-04T12:21:45Z

Issues Resolved

Closes #122

Check List

New functionality includes testing.
- All tests pass
Commits are signed per the DCO using --signoff

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.

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

AlibiZhenis · 2023-05-04T12:22:46Z

Since some function names were also changed in ML Commons, should I also change the method names?

codecov-commenter · 2023-05-04T12:38:05Z

Codecov Report

Merging #159 (3a7d72b) into main (3f15848) will increase coverage by 0.08%.
The diff coverage is 85.18%.

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   90.67%   90.76%   +0.08%     
==========================================
  Files          36       36              
  Lines        3989     4027      +38     
==========================================
+ Hits         3617     3655      +38     
  Misses        372      372

Impacted Files	Coverage Δ
opensearch_py_ml/ml_commons/ml_commons_client.py	`79.33% <83.67%> (+4.64%)`	⬆️
opensearch_py_ml/ml_commons/ml_common_utils.py	`100.00% <100.00%> (ø)`
opensearch_py_ml/ml_commons/model_uploader.py	`97.70% <100.00%> (ø)`

... and 1 file with indirect coverage changes

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

dhrubo-os · 2023-05-11T16:11:39Z

Can you join to the meeting?

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

dhrubo-os · 2023-05-16T15:33:56Z

codecov/patch is failing. Please check if you can address this or not. If you think some line's not possible to cover may be we can check how to skip those lines for testing may be?

AlibiZhenis · 2023-05-17T09:49:51Z

I don't quite understand the error. It says some lines were not covered in tests, but I'm not sure what it means. For example, it says line 130 in ml_commons_client.py was not covered in tests. But it's a part of a method that was used in tests. Can you clarify?

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

dhrubo-os · 2023-05-17T14:26:54Z

docs/source/reference/api/ml_commons_register_pretrained_api.rst

@@ -0,0 +1,6 @@
+Register Pretrained Model
+==================


We need to cover ====== all the way to Model.

dhrubo-os · 2023-05-17T14:27:08Z

docs/source/reference/mlcommons.rst

 Load Model
 ~~~~~~~~~~
 .. toctree::
   :maxdepth: 2

   api/ml_commons_load_api

+Deploy Model


dhrubo-os · 2023-05-17T14:39:34Z

opensearch_py_ml/ml_commons/ml_commons_client.py

+
+        # loading the model chunks from model index
+        if deploy_model:
+            self.deploy_model(model_id, wait_until_deployed=wait_until_deployed)


For line 129, it is saying coverage is missing.

Can you please check if there's any test method for register_model where we provide deploy_model = True?

I found one test method where deploy_model = False. May be we need to add another test method for deploy_model = True ?

dhrubo-os · 2023-05-17T14:41:29Z

opensearch_py_ml/ml_commons/ml_commons_client.py

@@ -138,53 +236,94 @@ def _send_model_info(self, model_meta_json: dict):
            if status["state"] != "CREATED":
                task_flag = True
        if not task_flag:
-            raise TimeoutError("Uploading model timed out")
+            raise TimeoutError("Model registration timed out")


I think this is hard to achieve in test. May be we can create a task and then set the end = 0 or low number so that task_flag remains false and then code coverage will be achieved.

TIMEOUT is a constant. To do this, we would need to add a TIMEOUT argument to the function. Should I do it?

oh I see. Hmm, let's add TODO in the code saying "need to add the test case later" for all these. We will revisit later.

Sure. Sorry for missing the meeting. I'l add the comment.

dhrubo-os · 2023-05-17T14:43:21Z

opensearch_py_ml/ml_commons/ml_commons_client.py

-                print("Model was loaded into memory only partially")
+            if model_state == "DEPLOYED":
+                print("Model deployed successfully")
+            elif model_state == "PARTIALLY_DEPLOYED":


This is hard to achieve. May be we can skip this for now.

dhrubo-os · 2023-05-17T14:43:44Z

opensearch_py_ml/ml_commons/ml_commons_client.py

            else:
-                raise Exception("Model load failed")
+                raise Exception("Model deployment failed")


I think this can be covered in test, right?

I'm not sure how I can force the deployment to fail

dhrubo-os · 2023-05-17T14:45:01Z

opensearch_py_ml/ml_commons/ml_commons_client.py

+            if model_state == "DEPLOYED":
+                print("Model deployed successfully")
+            elif model_state == "PARTIALLY_DEPLOYED":
+                print("Model deployed only partially")


skip this for now.

dhrubo-os · 2023-05-17T14:45:44Z

opensearch_py_ml/ml_commons/ml_commons_client.py

+
+        API_BODY = {}
+        if len(node_ids) > 0:
+            API_BODY["node_ids"] = node_ids


skip this for now also.

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

ylwu-amzn · 2023-05-18T17:55:39Z

opensearch_py_ml/ml_commons/ml_common_utils.py

@@ -6,7 +6,7 @@
 # GitHub history for details.

 ML_BASE_URI = "/_plugins/_ml"
-MODEL_UPLOAD_CHUNK_SIZE = 10_000_000
+MODEL_REGISTER_CHUNK_SIZE = 10_000_000


How about change to MODEL_CHUNK_SIZE_LIMIT

Or to match the MODEL_MAX_SIZE, another option is MODEL_CHUNK_MAX_SIZE ?

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

* renaming APIs for OS 2.7 Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * updating tests Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * linting Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * adding new methods and marking old ones as deprecated Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * fixing tests Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * updating documentation Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * partially fixing tests for codecov/patch Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * adding comments Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * change constant name Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * linting Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * fixing tests Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> --------- Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> (cherry picked from commit 7d01104)

* renaming APIs for OS 2.7 Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * updating tests Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * linting Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * adding new methods and marking old ones as deprecated Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * fixing tests Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * updating documentation Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * partially fixing tests for codecov/patch Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * adding comments Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * change constant name Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * linting Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> * fixing tests Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> --------- Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com> (cherry picked from commit 7d01104) Co-authored-by: Alibi Zhenis <92104549+AlibiZhenis@users.noreply.github.com>

AlibiZhenis added 3 commits May 4, 2023 19:43

renaming APIs for OS 2.7

2385d06

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

updating tests

5c769d5

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

linting

5b78d46

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

AlibiZhenis requested review from dhrubo-os, greaa-aws and ylwu-amzn as code owners May 4, 2023 12:21

adding new methods and marking old ones as deprecated

10aed7b

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

AlibiZhenis and others added 2 commits May 12, 2023 20:20

fixing tests

e808ce5

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

Merge branch 'opensearch-project:main' into renameAPI

Verified

This commit was created on github.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

146e8ab

dhrubo-os approved these changes May 14, 2023

View reviewed changes

Merge branch 'opensearch-project:main' into renameAPI

Verified

This commit was created on github.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

20a8fab

updating documentation

9470d1e

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

dhrubo-os reviewed May 17, 2023

View reviewed changes

partially fixing tests for codecov/patch

dafeef6

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

AlibiZhenis mentioned this pull request May 18, 2023

adding executeAPI #165

Merged

4 tasks

adding comments

75a765a

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

dhrubo-os approved these changes May 18, 2023

View reviewed changes

ylwu-amzn reviewed May 18, 2023

View reviewed changes

AlibiZhenis added 3 commits May 19, 2023 14:43

change constant name

f0e4b80

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

linting

86e7c51

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

fixing tests

3a7d72b

Signed-off-by: Alibi Zhenis <alibizhenis4@gmail.com>

dhrubo-os approved these changes May 19, 2023

View reviewed changes

greaa-aws approved these changes May 31, 2023

View reviewed changes

dhrubo-os merged commit 7d01104 into opensearch-project:main May 31, 2023

dhrubo-os added the backport 1.x label May 31, 2023

opensearch-trigger-bot bot mentioned this pull request May 31, 2023

[Backport 1.x] Rename APIs for model serving framework #167

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename APIs for model serving framework #159

Rename APIs for model serving framework #159

AlibiZhenis commented May 4, 2023 •

edited

Loading

AlibiZhenis commented May 4, 2023

codecov-commenter commented May 4, 2023 •

edited by codecov bot

Loading

dhrubo-os commented May 11, 2023

dhrubo-os commented May 16, 2023

AlibiZhenis commented May 17, 2023

dhrubo-os May 17, 2023

dhrubo-os May 17, 2023

dhrubo-os May 17, 2023

dhrubo-os May 17, 2023

AlibiZhenis May 18, 2023

dhrubo-os May 18, 2023

AlibiZhenis May 18, 2023

dhrubo-os May 17, 2023

dhrubo-os May 17, 2023

AlibiZhenis May 18, 2023

dhrubo-os May 17, 2023

dhrubo-os May 17, 2023

ylwu-amzn May 18, 2023

ylwu-amzn May 18, 2023

AlibiZhenis May 19, 2023

Rename APIs for model serving framework #159

Rename APIs for model serving framework #159

Conversation

AlibiZhenis commented May 4, 2023 • edited Loading

Issues Resolved

Check List

AlibiZhenis commented May 4, 2023

codecov-commenter commented May 4, 2023 • edited by codecov bot Loading

Codecov Report

dhrubo-os commented May 11, 2023

dhrubo-os commented May 16, 2023

AlibiZhenis commented May 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlibiZhenis commented May 4, 2023 •

edited

Loading

codecov-commenter commented May 4, 2023 •

edited by codecov bot

Loading