ml-commons
ml-commons copied to clipboard
populate time fields for connectors on return
Description
Currently any class that extends the AbstractConnector class has the fields createdTime and lastUpdatedTime set to null. The solution was creating setters on the connector interface and implementing on the abstract connector. Now on Indexing new connectors (Via the create API) the time fields will be instantiated. Two unit tests in UpdateConnectorTransportActionTests named UpdateConnectorTransportActionTests and testUpdateConnectorUpdatesHttpConnectorTimeFields
Related Issues
fixes #2890 Created Time and Last Updated Time are not implemented on Connector
Check List
- [X] New functionality includes testing.
- [X] Commits are signed per the DCO using
--signoff.
Changes (Amazon team can view this demo )
- Observe that connector creations using the Get Connector API now have a
created_time,last_updated_timea. creating a connector
curl -X POST \
-H "Content-Type: application/json" \
-d '{
"name": "Chat Gpt 3.5",
"description": "Brians Test Chatgpt",
"version": "1",
"protocol": "http",
"parameters": {
"endpoint": "api.openai.com",
"model": "gpt-3.5-turbo"
},
"credential": {
"openAI_key": "'"$BRIAN_OPENAI"'"
},
"actions": [
{
"action_type": "predict",
"method": "POST",
"url": "https://${parameters.endpoint}/v1/chat/completions",
"headers": {
"Authorization": "Bearer ${credential.openAI_key}"
},
"request_body": "{ \"model\": \"${parameters.model}\", \"messages\": ${parameters.messages} }"
}
]
}' \
localhost:9200/_plugins/_ml/connectors/_create
b. Inspecting changes via a get request `curl -XGET "http://localhost:9200/_plugins/_ml/connectors/lkf-3JEBz83ApvP3YOPy"`. Also note the followinf fields have been added `created_time,last_updated_time`
{
"name": "Chat Gpt 3.5",
"version": "1",
"description": "Brians Test Chatgpt",
"protocol": "http",
"parameters": {
"endpoint": "api.openai.com",
"model": "gpt-3.5-turbo"
},
"actions": [
{
"action_type": "PREDICT",
"method": "POST",
"url": "https://${parameters.endpoint}/v1/chat/completions",
"headers": {
"Authorization": "Bearer ${credential.openAI_key}"
},
"request_body": "{ \"model\": \"${parameters.model}\", \"messages\": ${parameters.messages} }"
}
],
"created_time": 1725989544074,
"last_updated_time": 1725989544074
}
-
Note how when updating the connector using the Update Connector API and then calling get it changes the the last updated time and leaves the created time the same a. Update the connector
curl -XPUT "http://localhost:9200/_plugins/_ml/connectors/lkf-3JEBz83ApvP3YOPy" -H "Content-Type: application/json' -d' { "description": "The new description of Brians chatGpt model" }"b. get the connector again and note the new time
curl -XGET "http://localhost:9200/_plugins/_ml/connectors/lkf-3JEBz83ApvP3YOPy"which yields
{
"name": "Chat Gpt 3.5",
"version": "1",
"description": "The new description of Brians chatGpt model",
"protocol": "http",
"parameters": {
"endpoint": "api.openai.com",
"model": "gpt-3.5-turbo"
},
"actions": [
{
"action_type": "PREDICT",
"method": "POST",
"url": "https://${parameters.endpoint}/v1/chat/completions",
"headers": {
"Authorization": "Bearer ${credential.openAI_key}"
},
"request_body": "{ \"model\": \"${parameters.model}\", \"messages\": ${parameters.messages} }"
}
],
"created_time": 1725989544074,
"last_updated_time": 1725989603171
}
- In summary the initial connector creation has the fields
"created_time": 1725989544074, "last_updated_time": 1725989544074and after the update its"created_time": 1725989544074, "last_updated_time": 1725989603171. You can see how it changed but the created_time stayed the same.
Testing
- Manual testing via using docker-compose (Was done in 2.x to get around version incompatibility and cherry picked to main branch)
- Spin up a docker compose file without the code changes (i.e. using the ml-commons code thats packed with openSearch)
- Create a connector and see it has been created via the get connectors API, (Observe no time fields)
- shut down the container run
./gradlew assembleto pack the code change into a zip file - add the following commands to the docker compose file
command: bash -c "bin/opensearch-plugin remove opensearch-skills; bin/opensearch-plugin remove opensearch-ml; bin/opensearch-plugin install --batch file:///<Path to the zip>/opensearch-ml-X.X.X.X-SNAPSHOT.zip; ./opensearch-docker-entrypoint.sh opensearch" - Get the connector that was done on step 2 observe no observed time
- Create a NEW connector and observe it has the time fields update it to see the updated time stamp works
- Two new unit tests were onboarded to simulate these changes in
UpdateConnectorTransportActionTestsnamed
testUpdateConnectorDoesNotUpdateHttpConnectorTimeFieldschecks that when creating a connector time fields arent populated we test this because we want to make sure that connectors dont get this field populated instead we want to make sure that the Transport layer sets this additionally when these fields are null we need to make sure the transport layer does not set the updated time on a update.testUpdateConnectorUpdatesHttpConnectorTimeFieldsChecks that time fields are instantiated and that after a update the updated time is bigger than the created time.
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.
Let's add more details in the description like how did you test?
Let's create a connector and invoke the get connector api to get the connector details to show the create and update timestamp
After that update the connector and then show the change of updated timestamp.
Hey @dhrubo-os Thanks for the feedback I updated my post based on your feedback. I uploaded a video that you can click on to see a visual of the written steps (2 mins) Its on the heading Changes
I also did a test on a cluster and the change works as well. (Amazonians can see this video as proof)
Hi Everyone thanks for your patience on this bug Ive updated this code with a final change that was discussed offline. Here is the final changes in a tabulated form. I would appreciate your review once more 😅 @dbwiddis, @rbhavna, @dhrubo-os, @ylwu-amzn
GET CONNECTOR (NO UPDATE)
| Connector | createdTime | lastUpdateTime |
|---|---|---|
| Old (was indexed prior to code change) | ❌ | ❌ |
| New (With code change) | ✅ | ✅ |
GET CONNECTOR (AFTER UPDATE)
| Connector | createdTime | lastUpdateTime |
|---|---|---|
| Old (was indexed prior to code change) | ❌ | ✅ |
| New (With code change) | ✅ | ✅ |
The backport to 2.x failed:
The process '/usr/bin/git' failed with exit code 1
To backport manually, run these commands in your terminal:
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-2922-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 88eaefdcfc2d1e37b8ec7bffd0e730840325a3f2
# Push it to GitHub
git push --set-upstream origin backport/backport-2922-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x
Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-2922-to-2.x.
@brianf-aws Backport PR for 2.x is failing. Can you please check why it's failing. And also please raise a backport PR for 2.x branch.