Skip to content
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 prefix strings to PUT model request #2449

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Conversation

davidkyle
Copy link
Member

The prefix_string option was added to the model configuration in #2363 but the PUT request does not use the same config and needs to be added explicitly.

The model config as defined in TrainedModels.ts cannot be used in the PUT request because it contains many fields that are automatically generated by the server and those fields should not be optional in the config but would have to be optional if used in the PUT request.

Closes #2448

@@ -429,9 +429,9 @@ export class TrainedModelPrefixStrings {
/**
* String prepended to input at ingest
*/
ingest: string
ingest?: string
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ingest and search params are optional, you can set one or the other or both or neither

Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
ml.clear_trained_model_deployment_cache 🟢 1/1 1/1
ml.close_job 🟢 64/64 63/63
ml.delete_calendar_event 🟢 4/4 4/4
ml.delete_calendar_job 🟢 3/3 3/3
ml.delete_calendar 🟢 5/5 5/5
ml.delete_data_frame_analytics 🟢 2/2 2/2
ml.delete_datafeed 🟢 3/3 3/3
ml.delete_expired_data 🟢 5/5 5/5
ml.delete_filter 🟢 27/27 27/27
ml.delete_forecast 🟢 3/3 3/3
ml.delete_job 🟢 47/47 47/47
ml.delete_model_snapshot 🟢 2/2 2/2
ml.delete_trained_model_alias 🟢 3/3 3/3
ml.delete_trained_model 🟢 4/4 4/4
ml.estimate_model_memory 🟢 16/16 16/16
ml.evaluate_data_frame 🟢 15/15 15/15
ml.explain_data_frame_analytics 🟢 7/7 7/7
ml.flush_job 🟢 15/15 15/15
ml.forecast 🟢 1/1 1/1
ml.get_buckets 🟢 14/14 14/14
ml.get_calendar_events 🟢 17/17 17/17
ml.get_calendars 🟢 17/17 17/17
ml.get_categories 🟢 12/12 12/12
ml.get_data_frame_analytics_stats 🟢 12/12 12/12
ml.get_data_frame_analytics 🔴 17/17 14/17
ml.get_datafeed_stats 🟢 27/27 27/27
ml.get_datafeeds 🔴 20/20 9/20
ml.get_filters 🟢 13/13 13/13
ml.get_influencers 🟢 11/11 11/11
ml.get_job_stats 🟢 32/32 32/32
ml.get_jobs 🔴 31/31 22/31
ml.get_memory_stats 🟢 6/6 6/6
ml.get_model_snapshot_upgrade_stats 🟢 3/3 3/3
ml.get_model_snapshots 🟢 18/18 18/18
ml.get_overall_buckets 🟢 16/16 15/15
ml.get_records 🟢 8/8 8/8
ml.get_trained_models_stats 🔴 17/17 10/17
ml.get_trained_models 🔴 31/37 32/37
ml.infer_trained_model 🔴 10/10 6/10
ml.info 🔴 10/10 2/10
ml.open_job 🟢 83/83 83/83
ml.post_calendar_events 🟢 21/21 21/21
ml.post_data 🔴 9/11 14/18
ml.preview_data_frame_analytics 🟢 3/3 3/3
ml.preview_datafeed 🔴 10/17 17/17
ml.put_calendar_job 🔴 11/12 12/12
ml.put_calendar 🟢 135/135 135/135
ml.put_data_frame_analytics 🔴 32/33 32/33
ml.put_datafeed 🔴 70/71 53/71
ml.put_filter 🟢 27/27 27/27
ml.put_job 🔴 219/227 223/225
ml.put_trained_model_alias 🟢 12/12 12/12
ml.put_trained_model_definition_part Missing test Missing test
ml.put_trained_model_vocabulary Missing test Missing test
ml.put_trained_model 🔴 14/15 6/15
ml.reset_job 🟢 2/2 2/2
ml.revert_model_snapshot 🟢 2/2 2/2
ml.set_upgrade_mode 🟢 6/6 6/6
ml.start_data_frame_analytics 🟢 1/1 1/1
ml.start_datafeed 🟢 24/24 24/24
ml.start_trained_model_deployment 🔴 13/13 3/13
ml.stop_data_frame_analytics 🟢 5/5 5/5
ml.stop_datafeed 🟢 17/17 17/17
ml.stop_trained_model_deployment 🟢 10/10 10/10
ml.update_data_frame_analytics 🟢 2/2 2/2
ml.update_datafeed 🔴 7/7 2/7
ml.update_filter 🟢 3/3 3/3
ml.update_job 🔴 4/5 5/5
ml.update_model_snapshot 🟢 3/3 3/3
ml.update_trained_model_deployment 🟠 Missing type Missing type
ml.upgrade_job_snapshot 🟢 3/3 3/3
ml.validate_detector 🟢 2/2 2/2
ml.validate 🟢 3/3 3/3

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM.

@davidkyle davidkyle merged commit f6ec736 into main Mar 13, 2024
6 checks passed
@davidkyle davidkyle deleted the prefix-strings-in-request branch March 13, 2024 13:34
Copy link
Contributor

The backport to 8.12 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-8.12 8.12
# Navigate to the new working tree
cd .worktrees/backport-8.12
# Create a new branch
git switch --create backport-2449-to-8.12
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f6ec736f5d7a3ab1ee4a4e5bbbc71b1c4ee478ca
# Push it to GitHub
git push --set-upstream origin backport-2449-to-8.12
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.12

Then, create a pull request where the base branch is 8.12 and the compare/head branch is backport-2449-to-8.12.

github-actions bot pushed a commit that referenced this pull request Mar 13, 2024
Follow up to #2363 where prefix_strings option was added to TrainedModelConfig

(cherry picked from commit f6ec736)
@pquentin
Copy link
Member

pquentin commented Mar 13, 2024

@davidkyle The 8.12 backport fails because #2363 was not backported to 8.12. Should it be?

If no, I can skip it in the 8.12 backport.

@davidkyle
Copy link
Member Author

Yes #2363 should have been backported because the Elasticsearch change was in 8.12

My mistake, I'll create the backports manually. Thanks @pquentin

davidkyle added a commit that referenced this pull request Mar 13, 2024
Follow up to #2363 where prefix_strings option was added to TrainedModelConfig
pquentin pushed a commit that referenced this pull request Mar 14, 2024
* Add prefix_string config option for ML models (#2363)

The prefix_strings option was added to support the E5 model in
elastic/elasticsearch#102089

* Add prefix_strings option to PUT model request (#2449)

Follow up to #2363 where prefix_strings option was added to TrainedModelConfig
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants