Added ML Model APIs #733

nathaliellenaa · 2024-12-13T21:51:49Z

Description

Added missing ML Model APIs.

Issues Resolved

Part of opensearch-project/opensearch-py#867.

ML Model APIs to add

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.

github-actions · 2024-12-13T21:54:05Z

Changes Analysis

Commit SHA: 7fa9b34
Comparing To SHA: 2b44e52

API Changes

Summary

├─┬Paths
│ ├──[➕] path (5331:3)
│ ├──[➕] path (5162:3)
│ ├──[➕] path (5405:3)
│ ├──[➕] path (5212:3)
│ ├──[➕] path (5349:3)
│ ├──[➕] path (5385:3)
│ ├──[➕] path (5426:3)
│ ├──[➕] path (5248:3)
│ ├──[➕] path (5445:3)
│ ├──[➕] path (5229:3)
│ ├─┬/_plugins/_ml/models/{model_id}
│ │ ├──[➕] get (5284:7)
│ │ ├──[➕] put (5299:7)
│ │ └─┬DELETE
│ │   └─┬Extensions
│ │     └──[➕] x-version-added (5271:24)
│ ├─┬/_plugins/_ml/models/_register
│ │ └─┬POST
│ │   ├─┬Requestbody
│ │   │ └─┬application/json
│ │   │   └─┬Schema
│ │   │     ├──[➖] required (27703:17)❌ 
│ │   │     ├─┬model_format
│ │   │     │ └──[🔀] $ref (34650:13)❌ 
│ │   │     ├─┬model_group_id
│ │   │     │ └──[🔀] $ref (34429:13)❌ 
│ │   │     └─┬version
│ │   │       └──[🔀] $ref (36018:13)❌ 
│ │   └─┬Extensions
│ │     └──[➕] x-version-added (5149:24)
│ ├─┬/_plugins/_ml/models/_search
│ │ ├──[➕] post (5197:7)
│ │ └─┬GET
│ │   └─┬Extensions
│ │     └──[➕] x-version-added (5183:24)
│ ├─┬/_plugins/_ml/models/{model_id}/_deploy
│ │ └─┬POST
│ │   └─┬Extensions
│ │     └──[➕] x-version-added (5319:24)
│ └─┬/_plugins/_ml/models/{model_id}/_undeploy
│   └─┬POST
│     ├──[➕] requestBody (28245:7)❌ 
│     └─┬Extensions
│       └──[➕] x-version-added (5371:24)
└─┬Components
  ├──[➕] requestBodies (27847:7)
  ├──[➕] requestBodies (28261:7)
  ├──[➕] requestBodies (28347:7)
  ├──[➕] requestBodies (28245:7)
  ├──[➕] requestBodies (27921:7)
  ├──[➕] requestBodies (28091:7)
  ├──[➕] requestBodies (27998:7)
  ├──[➕] responses (31740:7)
  ├──[➕] responses (31634:7)
  ├──[➕] responses (31467:7)
  ├──[➕] responses (31746:7)
  ├──[➕] responses (31424:7)
  ├──[➕] responses (31581:7)
  ├──[➕] responses (31607:7)
  ├──[➕] responses (31758:7)
  ├──[➕] responses (31569:7)
  ├──[➕] responses (31710:7)
  ├──[➕] requestBodies (28416:7)
  ├──[➕] requestBodies (28405:7)
  ├──[➕] parameters (23589:7)
  ├──[➕] parameters (23667:7)
  ├──[➕] parameters (23480:7)
  ├──[➕] parameters (23673:7)
  ├──[➕] parameters (23487:7)
  ├──[➕] parameters (23637:7)
  ├──[➕] parameters (23595:7)
  ├──[➕] parameters (23680:7)
  ├──[➕] parameters (23577:7)
  ├──[➕] schemas (55376:7)
  ├──[➕] schemas (56371:7)
  ├──[➕] schemas (55825:7)
  ├──[➕] schemas (55555:7)
  ├──[➕] schemas (55818:7)
  ├──[➕] schemas (56379:7)
  ├──[➕] schemas (55407:7)
  ├──[➕] schemas (55782:7)
  ├──[➕] schemas (56376:7)
  ├──[➕] schemas (56390:7)
  ├──[➕] schemas (34650:7)
  ├──[➕] schemas (55865:7)
  └─┬ml._common___Source
    └─┬model_format
      └──[🔀] $ref (34650:13)❌

Document Element	Total Changes	Breaking Changes
paths	23	5
components	41	1

❌ BREAKING Changes: 6 out of 64
Modifications: 4
Removals: 1
Additions: 59
Breaking Removals: 1
Breaking Modifications: 4
Breaking Additions: 1

Report

The full API changes report is available at: https://github.com/opensearch-project/opensearch-api-specification/actions/runs/12698226238/artifacts/2409613270

API Coverage

	Before	After	Δ
Covered (%)	630 (61.7 %)	643 (62.98 %)	13 (1.28 %)
Uncovered (%)	391 (38.3 %)	378 (37.02 %)	-13 (-1.28 %)
Unknown	43	43	0

github-actions · 2024-12-13T21:54:25Z

Spec Test Coverage Analysis

Total	Tested
573	572 (99.83 %)

dblock

Looking good. I have some smallish nits. Iterate to green (validation is failing, etc.)

CHANGELOG.md

tests/plugins/ml/ml/models.yaml

CHANGELOG.md

dblock · 2024-12-13T22:08:16Z

Btw, there's a whole bunch of others missing per #168. They don't all need to be done at the same time.

nathaliellenaa · 2024-12-13T22:15:40Z

Yes, I'm planning to create some small PRs based on the APIs category (model, model groups, connector, agents, etc.) I will also add more missing Model APIs to this PR.

spec/namespaces/ml.yaml

dhrubo-os · 2024-12-18T01:04:58Z

DCO is missing @nathaliellenaa

spec/namespaces/_core.yaml

dblock

Looking good. Some nits below.

In tests try to get rid of multiple-paths-detected: false as much as possible by moving setup into prologues and teardown into epilogues. You should only need that set if there's a need to call an unrelated API (e.g. wait on a task) to complete the test. Otherwise chapters should only have chapters for the API being tested.

CHANGELOG.md

spec/namespaces/ml.yaml

tests/plugins/ml/models/load.yaml

nathaliellenaa · 2024-12-24T20:23:29Z

The APIs POST /_plugins/_ml/models/{model_id}/{version}/_register and POST /_plugins/_ml/models/{model_id}/{version}/_upload will be deprecated since cx are now only using the POST /_plugins/_ml/models/_register API. Both APIs were non-functional in current and previous versions as well. I believe we should refrain from adding these APIs to the specification to avoid future deprecation efforts in the client side.

dblock

Good work.

Go through tests and make sure only the API being tested is in the chapters and everything else is in prologues or epilogues, and that the naming is consistent.

CHANGELOG.md

tests/plugins/ml/models/create_metadata.yaml

tests/plugins/ml/models/predict.yaml

tests/plugins/ml/models/undeploy_specific_models.yaml

tests/plugins/ml/models/unload.yaml

tests/plugins/ml/models/unload_specific_models.yaml

tests/plugins/ml/models/upload.yaml

dblock

Looks like tests fail consistently in CI, https://github.com/opensearch-project/opensearch-api-specification/actions/runs/12520890851/job/34927206366?pr=733, maybe needs longer retries or maybe there's a real problem.

dblock

To be consistent tests/plugins/ml/models should be tests/plugins/ml/ml/models. I know the double ml looks weird there, but the folder ml is the name of the test suite (like default) and from there the path is the API path, and all the APIs being tested are under _ml/models.

nathaliellenaa · 2024-12-30T18:13:15Z

Looks like tests fail consistently in CI, https://github.com/opensearch-project/opensearch-api-specification/actions/runs/12520890851/job/34927206366?pr=733, maybe needs longer retries or maybe there's a real problem.

The tests passed when get_completed_deploy_model_task is moved to chapters section, but they are failing when it is moved to prologues section. I'll look deeper into this issue.

Update: It seems that when the deploy task state is still CREATED, it immediately predicts the model without retrying it until the state is RUNNING or COMPLETED. I suspected that it's possibly picking up the state from registering a model previously, but I already verified that get_completed_deploy_model_task used the correct deploy model task id and task type.

@dblock Do you have any idea what causes this issue?

[INFO] => GET /_plugins/_ml/tasks/6I1wGZQBkjzF5RCKw0hU ({}) [application/json] 
[INFO] <= 200 (application/json; charset=UTF-8) | {
  "model_id": "541wGZQBkjzF5RCKdUiw",
  "task_type": "DEPLOY_MODEL",
  "function_name": "TEXT_EMBEDDING",
  "state": "CREATED",
  "worker_node": [
    "kLV6JVKsRpCfUABzKFFbsQ",
    "Wwv5YlwPRCeqFoTtk0-3Cg",
    "1Mel2oTKSH2Sxvy5W16QvQ",
    "VodBQqJ-TluOEBf4o5h72Q"
  ],
  "create_time": 1735593608020,
  "last_update_time": 1735593608020,
  "is_async": true
}
[INFO] $ {
  "outputs": {
    "model_id": "541wGZQBkjzF5RCKdUiw"
  }
}
[INFO] => POST /_plugins/_ml/models/541wGZQBkjzF5RCKdUiw/_predict ({}) [application/json] {
  "query_text": "The best selling book series in history is Harry Potter",
  "text_docs": [
    "Harry Potter is written by J.K. Rowling",
    "The Great Gatsby is a story of wealth and tragedy",
    "The Lord of the Rings is an epic high fantasy novel",
    "The best selling book series in history is Harry Potter"
  ]
}
[INFO] <= 400 (application/json) | {
  "error": {
    "root_cause": [
      {
        "type": "illegal_argument_exception",
        "reason": "Model not ready yet. Please deploy the model first."
      }
    ],
    "type": "illegal_argument_exception",
    "reason": "Model not ready yet. Please deploy the model first."
  },
  "status": 400
}

dblock · 2024-12-31T13:22:19Z

I don't believe retry can do this:

      until:
        - path: payload.state
          equal: COMPLETED

Retry schema is here. You want to wait till the response payload is COMPLETED, so:

response:
  payload:
    state: COMPLETED

Then retry will keep retrying until that's true.

    retry:
      count: 3
      wait: 10000

There's a bug in the schema, it should have prevented you from adding until which doesn't exist loudly. Open a bug please, and feel free to fix.

nathaliellenaa · 2024-12-31T22:28:19Z

Retry schema is here. You want to wait till the response payload is COMPLETED, so:
response:
  payload:
    state: COMPLETED

I checked the error and it still has the same issue on the get_completed_deploy_model_task, where it immediately predicts the model without waiting until deploy task state is COMPLETED. It seems that the prologues doesn't have response property (ref). Should the get_completed_deploy_model_task be moved to the chapters section then?

dblock · 2025-01-02T00:42:56Z

It seems that the prologues doesn't have response property (ref). Should the get_completed_deploy_model_task be moved to the chapters section then?

I think the right fix would be to add support for payload properties into the prologue. Want to give it a shot?

nathaliellenaa · 2025-01-02T17:37:14Z

I think the right fix would be to add support for payload properties into the prologue. Want to give it a shot?

Sure, I'll try this approach.

Update: I added the response payload properties into the prologue, but same issue still persists.

dblock · 2025-01-07T17:08:10Z

I think the right fix would be to add support for payload properties into the prologue. Want to give it a shot?

Sure, I'll try this approach.

Update: I added the response payload properties into the prologue, but same issue still persists.

Add that code here, I can try to help. Also since #767 we have logs from CI so it might be helpful to look at those.

dblock

We were not actually checking prologues and epilogues payloads, implemented in #772.

.github/workflows/test-spec.yml

dblock · 2025-01-09T17:08:19Z

@nathaliellenaa #772 was merged, rebase?

…h the tests Signed-off-by: Nathalie Jonathan <[email protected]>

…o models/search.yaml, resolved conflicts and updated CHANGELOG Signed-off-by: Nathalie Jonathan <[email protected]>

Signed-off-by: dblock <[email protected]> Signed-off-by: Nathalie Jonathan <[email protected]>

…project#732) * Fixed /_search/scroll. Signed-off-by: dblock <[email protected]> * Added tests for GET and POST /_search. Signed-off-by: dblock <[email protected]> * Added a test for GET /_search/pipeline and DELETE /_search/pipeline/{id}. Signed-off-by: dblock <[email protected]> * Added missing _search/point_in_time tests. Signed-off-by: dblock <[email protected]> --------- Signed-off-by: dblock <[email protected]> Signed-off-by: Nathalie Jonathan <[email protected]>

* Added tests for /_validate/query. Signed-off-by: dblock <[email protected]> * Added retry for opensearch-project#738. Signed-off-by: dblock <[email protected]> --------- Signed-off-by: dblock <[email protected]> Signed-off-by: Nathalie Jonathan <[email protected]>

Signed-off-by: Nathalie Jonathan <[email protected]>

…ata. Signed-off-by: Nathalie Jonathan <[email protected]>

…own to epilogues in predict.yaml and load.yaml, updated CHANGELOG format, updated API description, 'model_group_id' ID type, 'version' parameter, and made 'model_format' a type of its own in ml.yaml. Signed-off-by: Nathalie Jonathan <[email protected]>

…t for deprecated model metadata creation API. Signed-off-by: Nathalie Jonathan <[email protected]>

…pload to create_metadata.yaml, updated CHANGELOG. Signed-off-by: Nathalie Jonathan <[email protected]>

Signed-off-by: Nathalie Jonathan <[email protected]>

…cate map keys in ml._common.yaml, removed excluded parts and until property in the test files. Signed-off-by: Nathalie Jonathan <[email protected]>

…prologue, attempted to fix errors in predict.yaml. Signed-off-by: Nathalie Jonathan <[email protected]>

…y in prologues of predict.yaml, undeploy.yaml, unload.yaml, added version for ML Model APIs. Signed-off-by: Nathalie Jonathan <[email protected]>

nathaliellenaa · 2025-01-09T20:18:07Z

Looks like the implementation in #772 fixes the issue, thanks @dblock.

Signed-off-by: Nathalie Jonathan <[email protected]>

dblock · 2025-01-09T21:41:35Z

Looks like at least 1 flaky test: #776

nathaliellenaa requested review from dblock, harshavamsi, sachetalva, nhtruong, Xtansia, VachaShah, Tokesh and aabeshov as code owners December 13, 2024 21:51

dblock requested changes Dec 13, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

tests/plugins/ml/ml/models.yaml Outdated Show resolved Hide resolved

tests/plugins/ml/ml/models.yaml Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

nathaliellenaa changed the title ~~Added missing ML Model APIs to the spec along with the tests~~ Added ML Model APIs Dec 14, 2024

dhrubo-os reviewed Dec 16, 2024

View reviewed changes

spec/namespaces/ml.yaml Show resolved Hide resolved

dhrubo-os reviewed Dec 17, 2024

View reviewed changes

spec/namespaces/ml.yaml Show resolved Hide resolved

nathaliellenaa force-pushed the add-ml-models-api branch from 37dfa5f to 78cf49d Compare December 18, 2024 21:19

ylwu-amzn reviewed Dec 18, 2024

View reviewed changes

spec/namespaces/_core.yaml Outdated Show resolved Hide resolved

dblock reviewed Dec 19, 2024

View reviewed changes

dblock requested changes Dec 27, 2024

View reviewed changes

dblock approved these changes Dec 29, 2024

View reviewed changes

dblock requested changes Dec 29, 2024

View reviewed changes

dblock reviewed Dec 29, 2024

View reviewed changes

nathaliellenaa force-pushed the add-ml-models-api branch from 1e2619d to 9c72a0d Compare January 7, 2025 19:21

dblock mentioned this pull request Jan 7, 2025

Evaluate payload body in prologues and epilogues. #772

Merged

dblock reviewed Jan 7, 2025

View reviewed changes

.github/workflows/test-spec.yml Outdated Show resolved Hide resolved

nathaliellenaa and others added 16 commits January 9, 2025 09:49

Added ML Model APIs (get, search, update model) to the spec along wit…

504a01c

…h the tests Signed-off-by: Nathalie Jonathan <[email protected]>

Added ML Model APIs (undeploy, unload), moved test for search model t…

cb27dea

…o models/search.yaml, resolved conflicts and updated CHANGELOG Signed-off-by: Nathalie Jonathan <[email protected]>

Added support for using a certificate and key. (opensearch-project#731)

25826ce

Signed-off-by: dblock <[email protected]> Signed-off-by: Nathalie Jonathan <[email protected]>

Fixed lint and directory path error.

dd0648b

Signed-off-by: Nathalie Jonathan <[email protected]>

Added ML Model APIs (load, upload, create metadata).

5fb76cf

Signed-off-by: Nathalie Jonathan <[email protected]>

Added ML Model APIs (predict) and updated the test settings for metad…

2f8c590

…ata. Signed-off-by: Nathalie Jonathan <[email protected]>

Added ML Model APIs (chunk, register meta), updated CHANGELOG and tes…

880cd28

…t for deprecated model metadata creation API. Signed-off-by: Nathalie Jonathan <[email protected]>

Added ML Model APIs (upload chunk), moved test for deprecated chunk u…

689ed58

…pload to create_metadata.yaml, updated CHANGELOG. Signed-off-by: Nathalie Jonathan <[email protected]>

Applied suggestions from code review.

8244ad8

Signed-off-by: Nathalie Jonathan <[email protected]>

Merged main and resolve conflicts.

8254a21

Signed-off-by: Nathalie Jonathan <[email protected]>

Moved test models folder to tests/plugins/ml/ml/models, removed dupli…

8e2d9ff

…cate map keys in ml._common.yaml, removed excluded parts and until property in the test files. Signed-off-by: Nathalie Jonathan <[email protected]>

Fixed merge conflicts, added support for payload properties into the …

a83c308

…prologue, attempted to fix errors in predict.yaml. Signed-off-by: Nathalie Jonathan <[email protected]>

Rebase with main, fixed test-spec.yml content, added response propert…

5d536df

…y in prologues of predict.yaml, undeploy.yaml, unload.yaml, added version for ML Model APIs. Signed-off-by: Nathalie Jonathan <[email protected]>

nathaliellenaa force-pushed the add-ml-models-api branch from 9c72a0d to 5d536df Compare January 9, 2025 20:10

Modified response payload state from RUNNING to COMPLETED.

7fa9b34

Signed-off-by: Nathalie Jonathan <[email protected]>

dblock approved these changes Jan 9, 2025

View reviewed changes

dblock merged commit c1651ec into opensearch-project:main Jan 9, 2025
30 checks passed

nathaliellenaa deleted the add-ml-models-api branch January 9, 2025 21:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added ML Model APIs #733

Added ML Model APIs #733

nathaliellenaa commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

dblock left a comment •

edited

Loading

dblock commented Dec 13, 2024

nathaliellenaa commented Dec 13, 2024

dhrubo-os commented Dec 18, 2024

dblock left a comment

nathaliellenaa commented Dec 24, 2024

dblock left a comment

dblock left a comment

dblock left a comment

nathaliellenaa commented Dec 30, 2024 •

edited

Loading

dblock commented Dec 31, 2024

nathaliellenaa commented Dec 31, 2024

dblock commented Jan 2, 2025

nathaliellenaa commented Jan 2, 2025 •

edited

Loading

dblock commented Jan 7, 2025 •

edited

Loading

dblock left a comment

dblock commented Jan 9, 2025

nathaliellenaa commented Jan 9, 2025 •

edited

Loading

dblock commented Jan 9, 2025

Added ML Model APIs #733

Added ML Model APIs #733

Conversation

nathaliellenaa commented Dec 13, 2024 • edited Loading

Description

Issues Resolved

ML Model APIs to add

github-actions bot commented Dec 13, 2024 • edited Loading

Changes Analysis

API Changes

Summary

Report

API Coverage

github-actions bot commented Dec 13, 2024 • edited Loading

Spec Test Coverage Analysis

dblock left a comment • edited Loading

Choose a reason for hiding this comment

dblock commented Dec 13, 2024

nathaliellenaa commented Dec 13, 2024

dhrubo-os commented Dec 18, 2024

dblock left a comment

Choose a reason for hiding this comment

nathaliellenaa commented Dec 24, 2024

dblock left a comment

Choose a reason for hiding this comment

dblock left a comment

Choose a reason for hiding this comment

dblock left a comment

Choose a reason for hiding this comment

nathaliellenaa commented Dec 30, 2024 • edited Loading

dblock commented Dec 31, 2024

nathaliellenaa commented Dec 31, 2024

dblock commented Jan 2, 2025

nathaliellenaa commented Jan 2, 2025 • edited Loading

dblock commented Jan 7, 2025 • edited Loading

dblock left a comment

Choose a reason for hiding this comment

dblock commented Jan 9, 2025

nathaliellenaa commented Jan 9, 2025 • edited Loading

dblock commented Jan 9, 2025

nathaliellenaa commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

github-actions bot commented Dec 13, 2024 •

edited

Loading

dblock left a comment •

edited

Loading

nathaliellenaa commented Dec 30, 2024 •

edited

Loading

nathaliellenaa commented Jan 2, 2025 •

edited

Loading

dblock commented Jan 7, 2025 •

edited

Loading

nathaliellenaa commented Jan 9, 2025 •

edited

Loading