From 554bb84a3752a911696ac9668a606c0829f59ab6 Mon Sep 17 00:00:00 2001 From: vansangpfiev Date: Tue, 17 Sep 2024 08:12:03 +0700 Subject: [PATCH] feat: cortex models delete command (#1189) * feat: cortex models delete command * feat: add server API * feat: add server API * chore: add e2e tests for model delete --- engine/commands/model_del_cmd.cc | 56 +++++++++++++++++++++++ engine/commands/model_del_cmd.h | 11 +++++ engine/controllers/command_line_parser.cc | 12 ++++- engine/controllers/models.cc | 30 ++++++++++-- engine/controllers/models.h | 6 ++- engine/e2e-test/main.py | 1 + engine/e2e-test/test_cli_model_delete.py | 24 ++++++++++ engine/e2e-test/test_runner.py | 4 +- 8 files changed, 136 insertions(+), 8 deletions(-) create mode 100644 engine/commands/model_del_cmd.cc create mode 100644 engine/commands/model_del_cmd.h create mode 100644 engine/e2e-test/test_cli_model_delete.py 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 79080a104..b8baf3466 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 "services/engine_service.h" #include "utils/file_manager_utils.h" @@ -91,8 +92,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"); } diff --git a/engine/controllers/models.cc b/engine/controllers/models.cc index e4b6f4adc..1d3157fcb 100644 --- a/engine/controllers/models.cc +++ b/engine/controllers/models.cc @@ -1,13 +1,15 @@ #include "models.h" +#include "commands/model_del_cmd.h" #include "config/yaml_config.h" #include "trantor/utils/Logger.h" #include "utils/cortex_utils.h" #include "utils/file_manager_utils.h" #include "utils/model_callback_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; } @@ -168,4 +170,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..907ab3917 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 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