From 8a010083a8927c3973bf7392d79944c3e21c5327 Mon Sep 17 00:00:00 2001 From: Thuandz Date: Thu, 19 Sep 2024 14:20:34 +0700 Subject: [PATCH 1/6] model set alias command --- engine/commands/model_alias_cmd.cc | 23 +++++++ engine/commands/model_alias_cmd.h | 10 ++++ engine/controllers/command_line_parser.cc | 15 ++++- engine/controllers/models.cc | 60 +++++++++++++++++-- engine/controllers/models.h | 4 ++ .../test/components/test_modellist_utils.cc | 32 ++++++++++ engine/utils/modellist_utils.cc | 22 +++++++ engine/utils/modellist_utils.h | 1 + 8 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 engine/commands/model_alias_cmd.cc create mode 100644 engine/commands/model_alias_cmd.h diff --git a/engine/commands/model_alias_cmd.cc b/engine/commands/model_alias_cmd.cc new file mode 100644 index 000000000..45ffdc8d2 --- /dev/null +++ b/engine/commands/model_alias_cmd.cc @@ -0,0 +1,23 @@ +#include "model_alias_cmd.h" +#include "utils/modellist_utils.h" + +namespace commands { + +void ModelAliasCmd::Exec(const std::string& model_handle, + const std::string& model_alias) { + modellist_utils::ModelListUtils modellist_handler; + try { + if (modellist_handler.UpdateModelAlias(model_handle, model_alias)) { + CLI_LOG("Successfully set model alias '" + model_alias + + "' for modeID '" + model_handle + "'."); + } else { + CLI_LOG("Unable to set model alias for modelID '" + model_handle + + "': model alias '" + model_alias + "' or modelID is not unique!"); + } + } catch (const std::exception& e) { + CLI_LOG("Error when setting model alias ('" + model_alias + + "') for modelID '" + model_handle + "':" + e.what()); + } +} + +} // namespace commands \ No newline at end of file diff --git a/engine/commands/model_alias_cmd.h b/engine/commands/model_alias_cmd.h new file mode 100644 index 000000000..dcec44947 --- /dev/null +++ b/engine/commands/model_alias_cmd.h @@ -0,0 +1,10 @@ +#pragma once +#include +#include "utils/logging_utils.h" +namespace commands { + +class ModelAliasCmd { + public: + void Exec(const std::string& model_handle, const std::string& model_alias); +}; +} // namespace commands \ No newline at end of file diff --git a/engine/controllers/command_line_parser.cc b/engine/controllers/command_line_parser.cc index b55887ebd..a756dfd4d 100644 --- a/engine/controllers/command_line_parser.cc +++ b/engine/controllers/command_line_parser.cc @@ -6,6 +6,7 @@ #include "commands/engine_install_cmd.h" #include "commands/engine_list_cmd.h" #include "commands/engine_uninstall_cmd.h" +#include "commands/model_alias_cmd.h" #include "commands/model_del_cmd.h" #include "commands/model_get_cmd.h" #include "commands/model_list_cmd.h" @@ -152,6 +153,18 @@ bool CommandLineParser::SetupCommand(int argc, char** argv) { mdc.Exec(model_id); }); + std::string model_alias; + auto model_alias_cmd = + models_cmd->add_subcommand("alias", "Add alias name for a modelID"); + model_alias_cmd->add_option("--model_id", model_id, ""); + model_alias_cmd->require_option(); + model_alias_cmd->add_option("--model_alias", model_alias, ""); + model_alias_cmd->require_option(); + model_alias_cmd->callback([&model_id, &model_alias]() { + commands::ModelAliasCmd mdc; + mdc.Exec(model_id, model_alias); + }); + auto model_update_cmd = models_cmd->add_subcommand("update", "Update configuration of a model"); @@ -238,7 +251,7 @@ bool CommandLineParser::SetupCommand(int argc, char** argv) { auto ps_cmd = app_.add_subcommand("ps", "Show running models and their status"); ps_cmd->group(kSystemGroup); - + CLI11_PARSE(app_, argc, argv); if (argc == 1) { CLI_LOG(app_.help()); diff --git a/engine/controllers/models.cc b/engine/controllers/models.cc index 1d3157fcb..d8fede792 100644 --- a/engine/controllers/models.cc +++ b/engine/controllers/models.cc @@ -5,11 +5,11 @@ #include "utils/cortex_utils.h" #include "utils/file_manager_utils.h" #include "utils/model_callback_utils.h" +#include "utils/modellist_utils.h" - void - Models::PullModel( - const HttpRequestPtr& req, - std::function&& callback) const { +void Models::PullModel( + const HttpRequestPtr& req, + std::function&& callback) const { if (!http_util::HasFieldInReq(req, callback, "modelId")) { return; } @@ -192,4 +192,56 @@ void Models::DeleteModel(const HttpRequestPtr& req, resp->setStatusCode(k404NotFound); callback(resp); } +} + +void Models::SetModelAlias( + const HttpRequestPtr& req, + std::function&& callback) const { + if (!http_util::HasFieldInReq(req, callback, "modelId") || + !http_util::HasFieldInReq(req, callback, "modelAlias")) { + return; + } + auto model_handle = (*(req->getJsonObject())).get("modelId", "").asString(); + auto model_alias = (*(req->getJsonObject())).get("modelAlias", "").asString(); + LOG_DEBUG << "GetModel, Model handle: " << model_handle + << ", Model alias: " << model_alias; + + modellist_utils::ModelListUtils modellist_handler; + try { + if (modellist_handler.UpdateModelAlias(model_handle, model_alias)) { + std::string message = "Successfully set model alias '" + model_alias + + "' for modeID '" + model_handle + "'."; + LOG_INFO << message; + Json::Value ret; + ret["result"] = "OK"; + ret["modelHandle"] = model_handle; + ret["message"] = message; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k200OK); + callback(resp); + } else { + std::string message = "Unable to set model alias for modelID '" + + model_handle + "': model alias '" + model_alias + + "' or modelID is not unique!"; + LOG_ERROR << message; + Json::Value ret; + ret["result"] = "Set alias failed!"; + ret["modelHandle"] = model_handle; + ret["message"] = message; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k400BadRequest); + callback(resp); + } + } catch (const std::exception& e) { + std::string message = "Error when setting model alias ('" + model_alias + + "') for modelID '" + model_handle + "':" + e.what(); + LOG_ERROR << message; + Json::Value ret; + ret["result"] = "Set alias failed!"; + ret["modelHandle"] = model_handle; + ret["message"] = message; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k400BadRequest); + callback(resp); + } } \ No newline at end of file diff --git a/engine/controllers/models.h b/engine/controllers/models.h index 907ab3917..65a3641f3 100644 --- a/engine/controllers/models.h +++ b/engine/controllers/models.h @@ -16,6 +16,7 @@ class Models : public drogon::HttpController { METHOD_ADD(Models::ListModel, "/list", Get); METHOD_ADD(Models::GetModel, "/get", Post); METHOD_ADD(Models::DeleteModel, "/{1}", Delete); + METHOD_ADD(Models::SetModelAlias, "/alias", Post); METHOD_LIST_END void PullModel(const HttpRequestPtr& req, @@ -27,4 +28,7 @@ class Models : public drogon::HttpController { void DeleteModel(const HttpRequestPtr& req, std::function&& callback, const std::string& model_id) const; + void SetModelAlias( + const HttpRequestPtr& req, + std::function&& callback) const; }; \ No newline at end of file diff --git a/engine/test/components/test_modellist_utils.cc b/engine/test/components/test_modellist_utils.cc index cb46b2b70..2a7abc05a 100644 --- a/engine/test/components/test_modellist_utils.cc +++ b/engine/test/components/test_modellist_utils.cc @@ -88,4 +88,36 @@ TEST_F(ModelListUtilsTestSuite, TestPersistence) { EXPECT_EQ(retrieved_model.model_id, kTestModel.model_id); EXPECT_EQ(retrieved_model.author_repo_id, kTestModel.author_repo_id); model_list_.DeleteModelEntry("test_model_id"); +} + +TEST_F(ModelListUtilsTestSuite, TestUpdateModelAlias) { + // Add the test model + ASSERT_TRUE(model_list_.AddModelEntry(kTestModel)); + + // Test successful update + EXPECT_TRUE(model_list_.UpdateModelAlias("test_model_id", "new_test_alias")); + auto updated_model = model_list_.GetModelInfo("new_test_alias"); + EXPECT_EQ(updated_model.model_alias, "new_test_alias"); + EXPECT_EQ(updated_model.model_id, "test_model_id"); + + // Test update with non-existent model + EXPECT_FALSE(model_list_.UpdateModelAlias("non_existent_model", "another_alias")); + + // Test update with non-unique alias + modellist_utils::ModelEntry another_model = kTestModel; + another_model.model_id = "another_model_id"; + another_model.model_alias = "another_alias"; + ASSERT_TRUE(model_list_.AddModelEntry(another_model)); + + EXPECT_FALSE(model_list_.UpdateModelAlias("test_model_id", "another_alias")); + + // Test update using model alias instead of model ID + EXPECT_TRUE(model_list_.UpdateModelAlias("new_test_alias", "final_test_alias")); + updated_model = model_list_.GetModelInfo("final_test_alias"); + EXPECT_EQ(updated_model.model_alias, "final_test_alias"); + EXPECT_EQ(updated_model.model_id, "test_model_id"); + + // Clean up + model_list_.DeleteModelEntry("test_model_id"); + model_list_.DeleteModelEntry("another_model_id"); } \ No newline at end of file diff --git a/engine/utils/modellist_utils.cc b/engine/utils/modellist_utils.cc index a9527cbc1..2f20f5741 100644 --- a/engine/utils/modellist_utils.cc +++ b/engine/utils/modellist_utils.cc @@ -198,6 +198,28 @@ bool ModelListUtils::UpdateModelEntry(const std::string& identifier, return false; // Entry not found } +bool ModelListUtils::UpdateModelAlias(const std::string& model_id, + const std::string& new_model_alias) { + std::lock_guard lock(mutex_); + auto entries = LoadModelList(); + auto it = std::find_if( + entries.begin(), entries.end(), [&model_id](const ModelEntry& entry) { + return entry.model_id == model_id || entry.model_alias == model_id; + }); + bool check_alias_unique = std::none_of( + entries.begin(), entries.end(), [&](const ModelEntry& entry) { + return entry.model_id == new_model_alias || + entry.model_alias == new_model_alias; + }); + if (it != entries.end() && check_alias_unique) { + + (*it).model_alias = new_model_alias; + SaveModelList(entries); + return true; + } + return false; // Entry not found +} + bool ModelListUtils::DeleteModelEntry(const std::string& identifier) { std::lock_guard lock(mutex_); auto entries = LoadModelList(); diff --git a/engine/utils/modellist_utils.h b/engine/utils/modellist_utils.h index e8efab0d2..7625c264b 100644 --- a/engine/utils/modellist_utils.h +++ b/engine/utils/modellist_utils.h @@ -40,5 +40,6 @@ class ModelListUtils { bool UpdateModelEntry(const std::string& identifier, const ModelEntry& updated_entry); bool DeleteModelEntry(const std::string& identifier); + bool UpdateModelAlias(const std::string& model_id, const std::string& model_alias); }; } // namespace modellist_utils \ No newline at end of file From a01e645960a63e7e01776c42db493965efc3cd5c Mon Sep 17 00:00:00 2001 From: Thuandz Date: Thu, 19 Sep 2024 14:43:08 +0700 Subject: [PATCH 2/6] Update command description --- engine/controllers/command_line_parser.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/controllers/command_line_parser.cc b/engine/controllers/command_line_parser.cc index a756dfd4d..dfae9df58 100644 --- a/engine/controllers/command_line_parser.cc +++ b/engine/controllers/command_line_parser.cc @@ -156,9 +156,9 @@ bool CommandLineParser::SetupCommand(int argc, char** argv) { std::string model_alias; auto model_alias_cmd = models_cmd->add_subcommand("alias", "Add alias name for a modelID"); - model_alias_cmd->add_option("--model_id", model_id, ""); + model_alias_cmd->add_option("--model_id", model_id, "Can be modelID or model alias to identify model"); model_alias_cmd->require_option(); - model_alias_cmd->add_option("--model_alias", model_alias, ""); + model_alias_cmd->add_option("--new_model_alias", model_alias, "new alias to be set"); model_alias_cmd->require_option(); model_alias_cmd->callback([&model_id, &model_alias]() { commands::ModelAliasCmd mdc; From 7825f99ae9e595888367735c6ef9527bab75f7d0 Mon Sep 17 00:00:00 2001 From: Thuandz Date: Thu, 19 Sep 2024 15:15:27 +0700 Subject: [PATCH 3/6] set timeout of end2end test to None --- engine/e2e-test/main.py | 18 +++++++++--------- engine/e2e-test/test_cli_engine_install.py | 2 +- engine/e2e-test/test_cli_engine_uninstall.py | 4 ++-- engine/e2e-test/test_cli_model_delete.py | 3 +-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/engine/e2e-test/main.py b/engine/e2e-test/main.py index 1df424e65..d509b1292 100644 --- a/engine/e2e-test/main.py +++ b/engine/e2e-test/main.py @@ -1,14 +1,14 @@ import pytest -from test_api_engine_list import TestApiEngineList -from test_cli_engine_get import TestCliEngineGet -from test_cli_engine_install import TestCliEngineInstall -from test_cli_engine_list import TestCliEngineList -from test_cli_engine_uninstall import TestCliEngineUninstall +# from test_api_engine_list import TestApiEngineList +# from test_cli_engine_get import TestCliEngineGet +# from test_cli_engine_install import TestCliEngineInstall +# from test_cli_engine_list import TestCliEngineList +# from test_cli_engine_uninstall import TestCliEngineUninstall from test_cli_model_delete import TestCliModelDelete -from test_cli_model_pull_direct_url import TestCliModelPullDirectUrl -from test_cli_server_start import TestCliServerStart -from test_cortex_update import TestCortexUpdate -from test_create_log_folder import TestCreateLogFolder +# from test_cli_model_pull_direct_url import TestCliModelPullDirectUrl +# from test_cli_server_start import TestCliServerStart +# from test_cortex_update import TestCortexUpdate +# from test_create_log_folder import TestCreateLogFolder if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/engine/e2e-test/test_cli_engine_install.py b/engine/e2e-test/test_cli_engine_install.py index 89d49401d..e4e331030 100644 --- a/engine/e2e-test/test_cli_engine_install.py +++ b/engine/e2e-test/test_cli_engine_install.py @@ -8,7 +8,7 @@ class TestCliEngineInstall: def test_engines_install_llamacpp_should_be_successfully(self): exit_code, output, error = run( - "Install Engine", ["engines", "install", "cortex.llamacpp"], timeout=60 + "Install Engine", ["engines", "install", "cortex.llamacpp"], timeout=None ) assert "Start downloading" in output, "Should display downloading message" assert exit_code == 0, f"Install engine failed with error: {error}" diff --git a/engine/e2e-test/test_cli_engine_uninstall.py b/engine/e2e-test/test_cli_engine_uninstall.py index 525c0ad63..5e70ccf0c 100644 --- a/engine/e2e-test/test_cli_engine_uninstall.py +++ b/engine/e2e-test/test_cli_engine_uninstall.py @@ -8,7 +8,7 @@ class TestCliEngineUninstall: def setup_and_teardown(self): # Setup # Preinstall llamacpp engine - run("Install Engine", ["engines", "install", "cortex.llamacpp"],timeout=None) + run("Install Engine", ["engines", "install", "cortex.llamacpp"],timeout = None) yield @@ -18,7 +18,7 @@ def setup_and_teardown(self): def test_engines_uninstall_llamacpp_should_be_successfully(self): exit_code, output, error = run( - "Uninstall engine", ["engines", "uninstall", "cortex.llamacpp"] + "Uninstall engine", ["engines", "uninstall", "cortex.llamacpp"], timeout = None ) assert "Engine cortex.llamacpp uninstalled successfully!" in output assert exit_code == 0, f"Install engine failed with error: {error}" diff --git a/engine/e2e-test/test_cli_model_delete.py b/engine/e2e-test/test_cli_model_delete.py index e5373fac1..ce3750491 100644 --- a/engine/e2e-test/test_cli_model_delete.py +++ b/engine/e2e-test/test_cli_model_delete.py @@ -8,8 +8,7 @@ class TestCliModelDelete: def setup_and_teardown(self): # Setup # Pull model - run("Pull Model", ["pull", "tinyllama"], 120) - + run("Pull Model", ["pull", "tinyllama"], timeout=None) yield # Teardown From 5c156be4ced3cb37a9cf49fba492d2b73c479a02 Mon Sep 17 00:00:00 2001 From: nguyenhoangthuan99 <35255081+nguyenhoangthuan99@users.noreply.github.com> Date: Thu, 19 Sep 2024 15:50:21 +0700 Subject: [PATCH 4/6] Update main.py --- engine/e2e-test/main.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/engine/e2e-test/main.py b/engine/e2e-test/main.py index d509b1292..1df424e65 100644 --- a/engine/e2e-test/main.py +++ b/engine/e2e-test/main.py @@ -1,14 +1,14 @@ import pytest -# from test_api_engine_list import TestApiEngineList -# from test_cli_engine_get import TestCliEngineGet -# from test_cli_engine_install import TestCliEngineInstall -# from test_cli_engine_list import TestCliEngineList -# from test_cli_engine_uninstall import TestCliEngineUninstall +from test_api_engine_list import TestApiEngineList +from test_cli_engine_get import TestCliEngineGet +from test_cli_engine_install import TestCliEngineInstall +from test_cli_engine_list import TestCliEngineList +from test_cli_engine_uninstall import TestCliEngineUninstall from test_cli_model_delete import TestCliModelDelete -# from test_cli_model_pull_direct_url import TestCliModelPullDirectUrl -# from test_cli_server_start import TestCliServerStart -# from test_cortex_update import TestCortexUpdate -# from test_create_log_folder import TestCreateLogFolder +from test_cli_model_pull_direct_url import TestCliModelPullDirectUrl +from test_cli_server_start import TestCliServerStart +from test_cortex_update import TestCortexUpdate +from test_create_log_folder import TestCreateLogFolder if __name__ == "__main__": pytest.main([__file__, "-v"]) From edbf5ba1e582943472bbb857078e7a33a06061c6 Mon Sep 17 00:00:00 2001 From: nguyenhoangthuan99 <35255081+nguyenhoangthuan99@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:38:18 +0700 Subject: [PATCH 5/6] Update test_cli_engine_uninstall.py --- engine/e2e-test/test_cli_engine_uninstall.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/e2e-test/test_cli_engine_uninstall.py b/engine/e2e-test/test_cli_engine_uninstall.py index 5e70ccf0c..685e5387f 100644 --- a/engine/e2e-test/test_cli_engine_uninstall.py +++ b/engine/e2e-test/test_cli_engine_uninstall.py @@ -18,7 +18,7 @@ def setup_and_teardown(self): def test_engines_uninstall_llamacpp_should_be_successfully(self): exit_code, output, error = run( - "Uninstall engine", ["engines", "uninstall", "cortex.llamacpp"], timeout = None + "Uninstall engine", ["engines", "uninstall", "cortex.llamacpp"] ) assert "Engine cortex.llamacpp uninstalled successfully!" in output assert exit_code == 0, f"Install engine failed with error: {error}" From 9018dc6a3d67ff273fa6fad27b9d4f72dc47bde6 Mon Sep 17 00:00:00 2001 From: Thuandz Date: Fri, 20 Sep 2024 08:38:07 +0700 Subject: [PATCH 6/6] Fix comment --- engine/commands/model_alias_cmd.cc | 2 +- engine/controllers/command_line_parser.cc | 2 +- engine/controllers/models.cc | 2 +- engine/e2e-test/test_cli_engine_install.py | 4 ++-- engine/utils/modellist_utils.cc | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/commands/model_alias_cmd.cc b/engine/commands/model_alias_cmd.cc index 45ffdc8d2..2123d06cf 100644 --- a/engine/commands/model_alias_cmd.cc +++ b/engine/commands/model_alias_cmd.cc @@ -12,7 +12,7 @@ void ModelAliasCmd::Exec(const std::string& model_handle, "' for modeID '" + model_handle + "'."); } else { CLI_LOG("Unable to set model alias for modelID '" + model_handle + - "': model alias '" + model_alias + "' or modelID is not unique!"); + "': model alias '" + model_alias + "' is not unique!"); } } catch (const std::exception& e) { CLI_LOG("Error when setting model alias ('" + model_alias + diff --git a/engine/controllers/command_line_parser.cc b/engine/controllers/command_line_parser.cc index d85debeca..6357a11ff 100644 --- a/engine/controllers/command_line_parser.cc +++ b/engine/controllers/command_line_parser.cc @@ -156,7 +156,7 @@ bool CommandLineParser::SetupCommand(int argc, char** argv) { models_cmd->add_subcommand("alias", "Add alias name for a modelID"); model_alias_cmd->add_option("--model_id", model_id, "Can be modelID or model alias to identify model"); model_alias_cmd->require_option(); - model_alias_cmd->add_option("--new_model_alias", model_alias, "new alias to be set"); + model_alias_cmd->add_option("--alias", model_alias, "new alias to be set"); model_alias_cmd->require_option(); model_alias_cmd->callback([&model_id, &model_alias]() { commands::ModelAliasCmd mdc; diff --git a/engine/controllers/models.cc b/engine/controllers/models.cc index d8fede792..45a5bf60e 100644 --- a/engine/controllers/models.cc +++ b/engine/controllers/models.cc @@ -222,7 +222,7 @@ void Models::SetModelAlias( } else { std::string message = "Unable to set model alias for modelID '" + model_handle + "': model alias '" + model_alias + - "' or modelID is not unique!"; + "' is not unique!"; LOG_ERROR << message; Json::Value ret; ret["result"] = "Set alias failed!"; diff --git a/engine/e2e-test/test_cli_engine_install.py b/engine/e2e-test/test_cli_engine_install.py index e4e331030..dfb4e9599 100644 --- a/engine/e2e-test/test_cli_engine_install.py +++ b/engine/e2e-test/test_cli_engine_install.py @@ -8,7 +8,7 @@ class TestCliEngineInstall: def test_engines_install_llamacpp_should_be_successfully(self): exit_code, output, error = run( - "Install Engine", ["engines", "install", "cortex.llamacpp"], timeout=None + "Install Engine", ["engines", "install", "cortex.llamacpp"], timeout=600 ) assert "Start downloading" in output, "Should display downloading message" assert exit_code == 0, f"Install engine failed with error: {error}" @@ -31,7 +31,7 @@ def test_engines_install_onnx_on_tensorrt_should_be_failed(self): def test_engines_install_pre_release_llamacpp(self): exit_code, output, error = run( - "Install Engine", ["engines", "install", "cortex.llamacpp", "-v", "v0.1.29"], timeout=None + "Install Engine", ["engines", "install", "cortex.llamacpp", "-v", "v0.1.29"], timeout=600 ) assert "Start downloading" in output, "Should display downloading message" assert exit_code == 0, f"Install engine failed with error: {error}" diff --git a/engine/utils/modellist_utils.cc b/engine/utils/modellist_utils.cc index 2f20f5741..261bf58d5 100644 --- a/engine/utils/modellist_utils.cc +++ b/engine/utils/modellist_utils.cc @@ -208,7 +208,7 @@ bool ModelListUtils::UpdateModelAlias(const std::string& model_id, }); bool check_alias_unique = std::none_of( entries.begin(), entries.end(), [&](const ModelEntry& entry) { - return entry.model_id == new_model_alias || + return (entry.model_id == new_model_alias && entry.model_id != model_id) || entry.model_alias == new_model_alias; }); if (it != entries.end() && check_alias_unique) {