Set tenant ID for predict request #2619

arjunkumargiri · 2024-07-08T21:04:08Z

Description

Tenant ID changes to support predict API. With this change was able to successfully test predict API with remote model and multi tenancy enabled. Following changes are included in this PR:

Set tenant ID during predict request
MLModelManager to support getModel using tenantID. Removed passing SDKClient as input and moved it to class object.
Pass SDKClient to MLModelManager. TODO: SDKModule is being initialized twice. Move to a uniform mechanism to initialize SDKClientModule.
Bug fix in DDB client to pass tenant ID.

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
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: Arjun kumar Giri <arjung@amazon.com>

…ctor (opensearch-project#2546) * multi-tenancy for models (create, get, delete) Signed-off-by: Dhrubo Saha <dhrubo@amazon.com> * added update connector + update model Signed-off-by: Dhrubo Saha <dhrubo@amazon.com> --------- Signed-off-by: Dhrubo Saha <dhrubo@amazon.com> Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

…search-project#2489) * Add source map to GetDataObjectResponse Signed-off-by: Daniel Widdis <widdis@gmail.com> * Add test for map getter in clients Signed-off-by: Daniel Widdis <widdis@gmail.com> --------- Signed-off-by: Daniel Widdis <widdis@gmail.com> Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

…onnector Implementations (opensearch-project#2520) * Restore original exception handling expectations Signed-off-by: Daniel Widdis <widdis@gmail.com> * Add UpdateDataObject to interface and implementations Signed-off-by: Daniel Widdis <widdis@gmail.com> * Implement UpdateConnector action Signed-off-by: Daniel Widdis <widdis@gmail.com> * Move CompletionException handling to a common method Signed-off-by: Daniel Widdis <widdis@gmail.com> * Add tests for SDKClient exceptions refactored from Transport Action Signed-off-by: Daniel Widdis <widdis@gmail.com> --------- Signed-off-by: Daniel Widdis <widdis@gmail.com> Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

Signed-off-by: arjunkumargiri <142054468+arjunkumargiri@users.noreply.github.com>

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

Signed-off-by: arjunkumargiri <142054468+arjunkumargiri@users.noreply.github.com>

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

arjunkumargiri · 2024-07-11T16:29:32Z

LGTM.

Except integ tests are failing. Not sure why, is it the two client thing?

After this is merged, I'll pick up the TODOs with the injection:

* I'll change SdkClient from an interface to a concrete class, with an internal implementation as a field that it delegates to.  Then we can inject it in CreateComponents

* I'll have the class take settings when instantiating to choose which client to implement

* I'll update the SdkClientModule to be a util class to help with all the above

* I'll add a new client for Aws Sigv4 support per https://github.com/opensearch-project/opensearch-java/blob/main/guides/auth.md

Integ tests were failing due to transient exception. Rerunning integ tests succeeded.

Thanks for quick turnaround with SDKClient module refactoring in PR: #2638

dhrubo-os · 2024-07-11T16:35:06Z

plugin/src/main/java/org/opensearch/ml/action/controller/CreateControllerTransportAction.java

@@ -99,7 +99,8 @@ protected void doExecute(Task task, ActionRequest request, ActionListener<MLCrea

        try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
            ActionListener<MLCreateControllerResponse> wrappedListener = ActionListener.runBefore(actionListener, context::restore);
-            mlModelManager.getModel(modelId, null, excludes, ActionListener.wrap(mlModel -> {
+            // TODO: Add support for multi tenancy


This Controller actions will be disabled for multi-tenancy. @b4sjoo is working on this.

arjunkumargiri and others added 10 commits June 17, 2024 10:32

AWS DDB SDK client support for remote data store

068f0b0

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

AWS DDB SDK client support for remote data store

e9e209a

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

Addressed CR comment

20381be

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

Merge branch 'feature/multi_tenancy' into feature/multi_tenancy

02d0f35

Signed-off-by: arjunkumargiri <142054468+arjunkumargiri@users.noreply.github.com>

Added javadoc based on feedback

82bddc8

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

Merge branch 'feature/multi_tenancy' into feature/multi_tenancy

902f5c1

Signed-off-by: arjunkumargiri <142054468+arjunkumargiri@users.noreply.github.com>

Set tenant ID for predict request

8315ee3

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 21:04 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 21:04 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 21:04 — with GitHub Actions Failure

arjunkumargiri force-pushed the multi_tenancy_3 branch from f866e55 to 884d623 Compare July 8, 2024 22:54

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 22:54 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 22:54 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 22:54 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 22:54 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 22:54 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 8, 2024 22:54 — with GitHub Actions Failure

arjunkumargiri force-pushed the multi_tenancy_3 branch from 884d623 to 3249ac2 Compare July 9, 2024 04:54

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 9, 2024 04:55 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 9, 2024 04:55 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 10, 2024 22:21 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:05 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Error

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 00:06 — with GitHub Actions Failure

Addressed comments

53b4113

Signed-off-by: Arjun kumar Giri <arjung@amazon.com>

arjunkumargiri force-pushed the multi_tenancy_3 branch from 65b202e to 53b4113 Compare July 11, 2024 04:28

arjunkumargiri temporarily deployed to ml-commons-cicd-env July 11, 2024 04:28 — with GitHub Actions Inactive

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 04:28 — with GitHub Actions Failure

arjunkumargiri had a problem deploying to ml-commons-cicd-env July 11, 2024 04:28 — with GitHub Actions Error

dhrubo-os approved these changes Jul 11, 2024

View reviewed changes

dhrubo-os merged commit 8c80f43 into opensearch-project:feature/multi_tenancy Jul 11, 2024
7 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set tenant ID for predict request #2619

Set tenant ID for predict request #2619

arjunkumargiri commented Jul 8, 2024

arjunkumargiri commented Jul 11, 2024

dhrubo-os Jul 11, 2024

Set tenant ID for predict request #2619

Set tenant ID for predict request #2619

Conversation

arjunkumargiri commented Jul 8, 2024

Description

Check List

arjunkumargiri commented Jul 11, 2024

dhrubo-os Jul 11, 2024

Choose a reason for hiding this comment