-
Notifications
You must be signed in to change notification settings - Fork 138
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
add more stats: connector count, connector/config index status; fix model count bug #1180
Conversation
…odel count bug Signed-off-by: Yaliang Wu <[email protected]>
Codecov Report
@@ Coverage Diff @@
## 2.9 #1180 +/- ##
============================================
- Coverage 78.92% 78.75% -0.18%
- Complexity 2119 2121 +2
============================================
Files 167 167
Lines 8633 8676 +43
Branches 869 870 +1
============================================
+ Hits 6814 6833 +19
- Misses 1423 1448 +25
+ Partials 396 395 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
plugin/src/main/java/org/opensearch/ml/model/MLModelGroupManager.java
Outdated
Show resolved
Hide resolved
ml-algorithms/src/main/java/org/opensearch/ml/engine/encryptor/EncryptorImpl.java
Outdated
Show resolved
Hide resolved
@@ -77,7 +77,7 @@ public void createModelGroup(MLRegisterModelGroupInput input, ActionListener<Str | |||
new IllegalArgumentException( | |||
"The name you provided is already being used by another model with ID: " | |||
+ id | |||
+ ". Please provide a different name" | |||
+ ". Please provide a different name or add \"model_group_id\": \"lMPmr4kB4eSCtCCDmCDm\" to request body" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we update to Please provide a different model group name
. We can see people started having confusion: #1179
about: \"model_group_id\": \"lMPmr4kB4eSCtCCDmCDm\"
: Is this any static group id we are adding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed hard coded model id .
For Please provide a different model group name
, that looks also confusing as user just provide model name in the request body, not model group name,
POST _plugins/_ml/models/_upload
{
"name": "huggingface/sentence-transformers/all-MiniLM-L12-v2",
"version": "1.0.1",
"model_format": "TORCH_SCRIPT"
}
Signed-off-by: Yaliang Wu <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may add more granularity stats like connector/model count for sageMaker, etc, if necessary in the future.
Description
Add more stats:
Fix model count bug: local model may have multiple chunks, the model count number should not include model chunk.
Also fine tune some error message
Issues Resolved
[List any issues this PR will resolve]
Check List
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.