From b97e9ccf8194deac58b928c2871144ac7442a7ac Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Wed, 11 Sep 2024 10:40:24 +0700 Subject: [PATCH 1/4] feat: cortex models delete command --- engine/CMakeLists.txt | 2 +- engine/commands/model_del_cmd.cc | 56 +++++++++++++++++++++++ engine/commands/model_del_cmd.h | 11 +++++ engine/controllers/command_line_parser.cc | 12 ++++- 4 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 engine/commands/model_del_cmd.cc create mode 100644 engine/commands/model_del_cmd.h diff --git a/engine/CMakeLists.txt b/engine/CMakeLists.txt index 8a72e5040..18bf3eb7e 100644 --- a/engine/CMakeLists.txt +++ b/engine/CMakeLists.txt @@ -43,7 +43,7 @@ if(MSVC) $<$:/MT> #--| ) endif() - + if(LLAMA_CUDA) cmake_minimum_required(VERSION 3.17) diff --git a/engine/commands/model_del_cmd.cc b/engine/commands/model_del_cmd.cc new file mode 100644 index 000000000..f2023f5c1 --- /dev/null +++ b/engine/commands/model_del_cmd.cc @@ -0,0 +1,56 @@ +#include "model_del_cmd.h" +#include "cmd_info.h" +#include "config/yaml_config.h" +#include "utils/file_manager_utils.h" + +namespace commands { +bool ModelDelCmd::Exec(const std::string& model_id) { + // TODO this implentation may be changed after we have a decision + // on https://github.com/janhq/cortex.cpp/issues/1154 but the logic should be similar + CmdInfo ci(model_id); + std::string model_file = + ci.branch == "main" ? ci.model_name : ci.model_name + "-" + ci.branch; + auto models_path = file_manager_utils::GetModelsContainerPath(); + if (std::filesystem::exists(models_path) && + std::filesystem::is_directory(models_path)) { + // Iterate through directory + for (const auto& entry : std::filesystem::directory_iterator(models_path)) { + if (entry.is_regular_file() && entry.path().extension() == ".yaml") { + try { + config::YamlHandler handler; + handler.ModelConfigFromFile(entry.path().string()); + auto cfg = handler.GetModelConfig(); + if (entry.path().stem().string() == model_file) { + // Delete data + if (cfg.files.size() > 0) { + std::filesystem::path f(cfg.files[0]); + auto rel = std::filesystem::relative(f, models_path); + // Only delete model data if it is stored in our models folder + if (!rel.empty()) { + if (cfg.engine == "cortex.llamacpp") { + std::filesystem::remove_all(f.parent_path()); + } else { + std::filesystem::remove_all(f); + } + } + } + + // Delete yaml file + std::filesystem::remove(entry); + CLI_LOG("The model " << model_id << " was deleted"); + return true; + } + } catch (const std::exception& e) { + CTL_WRN("Error reading yaml file '" << entry.path().string() + << "': " << e.what()); + return false; + } + } + } + } + + CLI_LOG("Model does not exist: " << model_id); + + return false; +} +} // namespace commands \ No newline at end of file diff --git a/engine/commands/model_del_cmd.h b/engine/commands/model_del_cmd.h new file mode 100644 index 000000000..0dd41f74e --- /dev/null +++ b/engine/commands/model_del_cmd.h @@ -0,0 +1,11 @@ +#pragma once + +#include + +namespace commands { + +class ModelDelCmd { + public: + bool Exec(const std::string& model_id); +}; +} \ No newline at end of file diff --git a/engine/controllers/command_line_parser.cc b/engine/controllers/command_line_parser.cc index 987aa693a..0adc8d490 100644 --- a/engine/controllers/command_line_parser.cc +++ b/engine/controllers/command_line_parser.cc @@ -13,6 +13,7 @@ #include "commands/model_stop_cmd.h" #include "commands/run_cmd.h" #include "commands/server_stop_cmd.h" +#include "commands/model_del_cmd.h" #include "config/yaml_config.h" #include "httplib.h" #include "services/engine_service.h" @@ -92,8 +93,15 @@ bool CommandLineParser::SetupCommand(int argc, char** argv) { command.Exec(); }); - auto remove_cmd = - models_cmd->add_subcommand("remove", "Remove a model by ID locally"); + auto model_del_cmd = + models_cmd->add_subcommand("delete", "Delete a model by ID locally"); + model_del_cmd->add_option("model_id", model_id, ""); + + model_del_cmd->callback([&model_id]() { + commands::ModelDelCmd mdc; + mdc.Exec(model_id); + }); + auto update_cmd = models_cmd->add_subcommand("update", "Update configuration of a model"); } From a67a35b727b29cc1f9e1380135f9abc9f7f5811d Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Mon, 16 Sep 2024 10:34:44 +0700 Subject: [PATCH 2/4] feat: add server API --- engine/controllers/models.cc | 65 ++++++++++++++++++++++++++++++++++++ engine/controllers/models.h | 6 +++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/engine/controllers/models.cc b/engine/controllers/models.cc index e4b6f4adc..3967a8f36 100644 --- a/engine/controllers/models.cc +++ b/engine/controllers/models.cc @@ -1,4 +1,5 @@ #include "models.h" +#include "commands/cmd_info.h" #include "config/yaml_config.h" #include "trantor/utils/Logger.h" #include "utils/cortex_utils.h" @@ -168,4 +169,68 @@ void Models::GetModel( auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); resp->setStatusCode(k200OK); callback(resp); +} + +void Models::DeleteModel( + const HttpRequestPtr& req, + std::function&& callback) const { + if (!http_util::HasFieldInReq(req, callback, "modelId")) { + return; + } + auto model_handle = (*(req->getJsonObject())).get("modelId", "").asString(); + LOG_DEBUG << "DeleteModel, Model handle: " << model_handle; + commands::CmdInfo ci(model_handle); + std::string model_file = + ci.branch == "main" ? ci.model_name : ci.model_name + "-" + ci.branch; + auto models_path = file_manager_utils::GetModelsContainerPath(); + if (std::filesystem::exists(models_path) && + std::filesystem::is_directory(models_path)) { + // Iterate through directory + for (const auto& entry : std::filesystem::directory_iterator(models_path)) { + if (entry.is_regular_file() && entry.path().extension() == ".yaml") { + try { + config::YamlHandler handler; + handler.ModelConfigFromFile(entry.path().string()); + auto cfg = handler.GetModelConfig(); + if (entry.path().stem().string() == model_file) { + // Delete data + if (cfg.files.size() > 0) { + std::filesystem::path f(cfg.files[0]); + auto rel = std::filesystem::relative(f, models_path); + // Only delete model data if it is stored in our models folder + if (!rel.empty()) { + if (cfg.engine == "cortex.llamacpp") { + std::filesystem::remove_all(f.parent_path()); + } else { + std::filesystem::remove_all(f); + } + } + } + + // Delete yaml file + std::filesystem::remove(entry); + LOG_INFO << "The model " << model_handle << " was deleted"; + Json::Value ret; + ret["result"] = "OK"; + ret["modelHandle"] = model_handle; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k200OK); + callback(resp); + return; + } + } catch (const std::exception& e) { + LOG_WARN << "Error reading yaml file '" << entry.path().string() + << "': " << e.what(); + } + } + } + } + + LOG_INFO << "Model does not exist: " << model_handle; + Json::Value ret; + ret["result"] = "Not Found"; + ret["modelHandle"] = model_handle; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k404NotFound); + callback(resp); } \ No newline at end of file diff --git a/engine/controllers/models.h b/engine/controllers/models.h index 789ce1398..e3f25425e 100644 --- a/engine/controllers/models.h +++ b/engine/controllers/models.h @@ -15,6 +15,7 @@ class Models : public drogon::HttpController { METHOD_ADD(Models::PullModel, "/pull", Post); METHOD_ADD(Models::ListModel, "/list", Get); METHOD_ADD(Models::GetModel, "/get", Post); + METHOD_ADD(Models::DeleteModel, "/delete", Post); METHOD_LIST_END void PullModel(const HttpRequestPtr& req, @@ -22,5 +23,8 @@ class Models : public drogon::HttpController { void ListModel(const HttpRequestPtr& req, std::function&& callback) const; void GetModel(const HttpRequestPtr& req, - std::function&& callback) const; + std::function&& callback) const; + void DeleteModel( + const HttpRequestPtr& req, + std::function&& callback) const; }; \ No newline at end of file From 5080fdd645bc2d82bdc7794d04adeb4e0e874c01 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Mon, 16 Sep 2024 10:34:44 +0700 Subject: [PATCH 3/4] feat: add server API --- engine/controllers/models.cc | 23 +++++++++++++++++++++++ engine/controllers/models.h | 6 +++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/engine/controllers/models.cc b/engine/controllers/models.cc index e4b6f4adc..935447b37 100644 --- a/engine/controllers/models.cc +++ b/engine/controllers/models.cc @@ -1,4 +1,5 @@ #include "models.h" +#include "commands/model_del_cmd.h" #include "config/yaml_config.h" #include "trantor/utils/Logger.h" #include "utils/cortex_utils.h" @@ -168,4 +169,26 @@ void Models::GetModel( auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); resp->setStatusCode(k200OK); callback(resp); +} + +void Models::DeleteModel(const HttpRequestPtr& req, + std::function&& callback, + const std::string& model_id) const { + LOG_DEBUG << "DeleteModel, Model handle: " << model_id; + commands::ModelDelCmd mdc; + if (mdc.Exec(model_id)) { + Json::Value ret; + ret["result"] = "OK"; + ret["modelHandle"] = model_id; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k200OK); + callback(resp); + } else { + Json::Value ret; + ret["result"] = "Not Found"; + ret["modelHandle"] = model_id; + auto resp = cortex_utils::CreateCortexHttpJsonResponse(ret); + resp->setStatusCode(k404NotFound); + callback(resp); + } } \ No newline at end of file diff --git a/engine/controllers/models.h b/engine/controllers/models.h index 789ce1398..04f0d44d8 100644 --- a/engine/controllers/models.h +++ b/engine/controllers/models.h @@ -15,6 +15,7 @@ class Models : public drogon::HttpController { METHOD_ADD(Models::PullModel, "/pull", Post); METHOD_ADD(Models::ListModel, "/list", Get); METHOD_ADD(Models::GetModel, "/get", Post); + METHOD_ADD(Models::DeleteModel, "/{1}", Delete); METHOD_LIST_END void PullModel(const HttpRequestPtr& req, @@ -22,5 +23,8 @@ class Models : public drogon::HttpController { void ListModel(const HttpRequestPtr& req, std::function&& callback) const; void GetModel(const HttpRequestPtr& req, - std::function&& callback) const; + std::function&& callback) const; + void DeleteModel( + const HttpRequestPtr& req, + std::function&& callback, const std::string& model_id) const; }; \ No newline at end of file From 21a993f58007aae3c5c60508b04910abb991ae03 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Mon, 16 Sep 2024 13:11:54 +0700 Subject: [PATCH 4/4] chore: add e2e tests for model delete --- engine/e2e-test/main.py | 1 + engine/e2e-test/test_cli_model_delete.py | 24 ++++++++++++++++++++++++ engine/e2e-test/test_runner.py | 4 ++-- 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 engine/e2e-test/test_cli_model_delete.py diff --git a/engine/e2e-test/main.py b/engine/e2e-test/main.py index d6af1522c..da1197c33 100644 --- a/engine/e2e-test/main.py +++ b/engine/e2e-test/main.py @@ -7,6 +7,7 @@ from test_cortex_update import TestCortexUpdate from test_cli_server_start import TestCliServerStart from test_create_log_folder import TestCreateLogFolder +from test_cli_model_delete import TestCliModelDelete if __name__ == "__main__": pytest.main([__file__, "-v"]) diff --git a/engine/e2e-test/test_cli_model_delete.py b/engine/e2e-test/test_cli_model_delete.py new file mode 100644 index 000000000..e5373fac1 --- /dev/null +++ b/engine/e2e-test/test_cli_model_delete.py @@ -0,0 +1,24 @@ +import pytest +from test_runner import run + + +class TestCliModelDelete: + + @pytest.fixture(autouse=True) + def setup_and_teardown(self): + # Setup + # Pull model + run("Pull Model", ["pull", "tinyllama"], 120) + + yield + + # Teardown + # Clean up + run("Delete model", ["models", "delete", "tinyllama"]) + + def test_models_delete_should_be_successful(self): + exit_code, output, error = run( + "Delete model", ["models", "delete", "tinyllama"] + ) + assert "The model tinyllama was deleted" in output + assert exit_code == 0, f"Model does not exist: {error}" diff --git a/engine/e2e-test/test_runner.py b/engine/e2e-test/test_runner.py index 4d3390b54..bc89fb836 100644 --- a/engine/e2e-test/test_runner.py +++ b/engine/e2e-test/test_runner.py @@ -24,13 +24,13 @@ def getExecutablePath() -> str: # Execute a command -def run(test_name: str, arguments: List[str]): +def run(test_name: str, arguments: List[str], timeout_sec = 5): executable_path = getExecutablePath() print("Running:", test_name) print("Command:", [executable_path] + arguments) result = subprocess.run( - [executable_path] + arguments, capture_output=True, text=True, timeout=timeout + [executable_path] + arguments, capture_output=True, text=True, timeout=timeout_sec ) return result.returncode, result.stdout, result.stderr