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

feat: cortex models delete command #1189

Merged
merged 7 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions engine/commands/model_del_cmd.cc
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions engine/commands/model_del_cmd.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#pragma once

#include <string>

namespace commands {

class ModelDelCmd {
public:
bool Exec(const std::string& model_id);
};
}
12 changes: 10 additions & 2 deletions engine/controllers/command_line_parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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");
}
Expand Down
30 changes: 27 additions & 3 deletions engine/controllers/models.cc
Original file line number Diff line number Diff line change
@@ -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<void(const HttpResponsePtr&)>&& callback) const {
void
Models::PullModel(
const HttpRequestPtr& req,
std::function<void(const HttpResponsePtr&)>&& callback) const {
if (!http_util::HasFieldInReq(req, callback, "modelId")) {
return;
}
Expand Down Expand Up @@ -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<void(const HttpResponsePtr&)>&& 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);
}
}
6 changes: 5 additions & 1 deletion engine/controllers/models.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@ class Models : public drogon::HttpController<Models> {
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,
std::function<void(const HttpResponsePtr&)>&& callback) const;
void ListModel(const HttpRequestPtr& req,
std::function<void(const HttpResponsePtr&)>&& callback) const;
void GetModel(const HttpRequestPtr& req,
std::function<void(const HttpResponsePtr&)>&& callback) const;
std::function<void(const HttpResponsePtr&)>&& callback) const;
void DeleteModel(const HttpRequestPtr& req,
std::function<void(const HttpResponsePtr&)>&& callback,
const std::string& model_id) const;
};
1 change: 1 addition & 0 deletions engine/e2e-test/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
24 changes: 24 additions & 0 deletions engine/e2e-test/test_cli_model_delete.py
Original file line number Diff line number Diff line change
@@ -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}"
4 changes: 2 additions & 2 deletions engine/e2e-test/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading