-
Notifications
You must be signed in to change notification settings - Fork 37
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
[FEATURE] RegisterModelStep / DeployModelStep Improvements #108
Comments
I'm unclear what this means. I thought we were taking a dependency on the MLClient separately, and we don't actually call any APIs during installation of the plugin. What exactly is the problem? |
|
I have similar question
@owaiskazi19 , what's the problem? |
@ylwu-amzn When invoking register API though mlClient here. Getting the below ClassCastException [2023-10-25T20:13:30,038][ERROR][o.o.f.t.ProvisionWorkflowTransportAction] [ip-172-31-56-214] Provisioning failed for workflow rlt4aIsB-BC8dYg-Eww9 : java.util.concurrent.ExecutionException: org.opensearch.flowframework.exception.FlowFrameworkException: class org.opensearch.ml.common.transport.register.MLRegisterModelResponse cannot be cast to class org.opensearch.ml.common.transport.register.MLRegisterModelResponse (org.opensearch.ml.common.transport.register.MLRegisterModelResponse is in unnamed module of loader java.net.FactoryURLClassLoader @475f5672; org.opensearch.ml.common.transport.register.MLRegisterModelResponse is in unnamed module of loader java.net.FactoryURLClassLoader @320be73) |
Specifically the same class (
|
@owaiskazi19 did you look into the different thread pools being used? |
Yes, tried using generic first and then our own. Same error for both. |
For the 2nd issue. There's no fix solution for it. But that's not the case for our plugin, we will have use cases which can rely on other plugins, at that time our plugin shouldn't have a strict requirement of installing ml-commons before our plugin. |
Agreed, I think the best way forward now would be to introduce a mechanism that can check the installed plugins during bootstrap of the OpenSearch process, probably during the instantiation of this |
Created #126 for the 2nd one. We can close this one in. |
Is your feature request related to a problem?
In an effort to address any issues/gaps with the current step implementations, this issue is to track necessary modifications for the
RegisterModelStep
andDeployModelStep
Context
In order to use the register and deploy model APIs, it is first necessary to install the ML Commons plugin prior to installing the Flow Framework Plugin.
These steps were tested prior to the availability of the
CreateConnectorStep
, so the create connector API was invoked to get theconnector_id
prior to invoking theRestCreateWorkflowAction
, since it is a necessary input of theRegisterModelStep
The following use case template was used to test out register and deploy model :
Logs for creating and provisioning a workflow :
OpenSearch Logs
Action Items
model_id
input for theDeployModelStep
, which is an output of theRegisterModelStep
. Currently, theDeployModelStep
works fine without specifying themodel_id
input on the use case template, but as a builder, it will be confusing without a placeholder. It will be necessary to determine what format dependent inputs for each step should look like on a use case template (Handled by Modifies use case template format and adds graph validation when provisioning #119)RegisterModelStep
andDeployModelStep
since these steps utilize that plugin's APIs. We can set ML Commons Plugin to be anextendedPlugin
within ourbuild.gradle
so that installation of theFlowFrameworkPlugin
requires ML Commons to be installed first, but this would require the installation of that plugin even for use cases that don't require those associatedWorkflowSteps
. ( @dbwiddis I think its important for us to discuss how we want to handle dependencies on different plugins)RegisterModelStep
completes exceptionally due to the above error, though the logs indicate that the model has been successfully registered on the ML Commons side. Fixed by fix register client API ml-commons#1560The text was updated successfully, but these errors were encountered: