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

fix: Isolate stdout from the API server and CLI #1526

Merged
merged 2 commits into from
Oct 22, 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
4 changes: 2 additions & 2 deletions engine/controllers/engines.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void Engines::InstallEngine(
}

auto version{"latest"};
auto result = engine_service_->InstallEngine(engine, version);
auto result = engine_service_->InstallEngineAsync(engine, version);
if (result.has_error()) {
Json::Value res;
res["message"] = result.error();
Expand All @@ -30,7 +30,7 @@ void Engines::InstallEngine(
callback(resp);
} else {
Json::Value res;
res["message"] = "Engine " + engine + " installed successfully!";
res["message"] = "Engine " + engine + " starts installing!";
auto resp = cortex_utils::CreateCortexHttpJsonResponse(res);
resp->setStatusCode(k200OK);
callback(resp);
Expand Down
3 changes: 3 additions & 0 deletions engine/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@ int main(int argc, char* argv[]) {
return 1;
}

// avoid printing logs to terminal
is_server = true;

std::optional<int> server_port;
for (int i = 0; i < argc; i++) {
if (strcmp(argv[i], "--config_file_path") == 0) {
Expand Down
104 changes: 68 additions & 36 deletions engine/services/engine_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,23 @@ cpp::result<bool, std::string> EngineService::InstallEngine(
}
}

cpp::result<bool, std::string> EngineService::InstallEngineAsync(
const std::string& engine, const std::string& version,
const std::string& src) {
// Although this function is called async, only download tasks are performed async
// TODO(sang) better handler for unzip and download scenarios
auto ne = NormalizeEngine(engine);
if (!src.empty()) {
return UnzipEngine(ne, version, src);
} else {
auto result = DownloadEngine(ne, version, true /*async*/);
if (result.has_error()) {
return result;
}
return DownloadCuda(ne, true /*async*/);
}
}

cpp::result<bool, std::string> EngineService::UnzipEngine(
const std::string& engine, const std::string& version,
const std::string& path) {
Expand Down Expand Up @@ -222,7 +239,7 @@ cpp::result<bool, std::string> EngineService::UninstallEngine(
}

cpp::result<bool, std::string> EngineService::DownloadEngine(
const std::string& engine, const std::string& version) {
const std::string& engine, const std::string& version, bool async) {

// Check if GITHUB_TOKEN env exist
const char* github_token = std::getenv("GITHUB_TOKEN");
Expand Down Expand Up @@ -320,27 +337,34 @@ cpp::result<bool, std::string> EngineService::DownloadEngine(
.localPath = local_path,
}}}};

return download_service_->AddDownloadTask(
downloadTask, [](const DownloadTask& finishedTask) {
// try to unzip the downloaded file
CTL_INF("Engine zip path: "
<< finishedTask.items[0].localPath.string());

std::filesystem::path extract_path =
finishedTask.items[0].localPath.parent_path().parent_path();

archive_utils::ExtractArchive(
finishedTask.items[0].localPath.string(),
extract_path.string());

// remove the downloaded file
try {
std::filesystem::remove(finishedTask.items[0].localPath);
} catch (const std::exception& e) {
CTL_WRN("Could not delete file: " << e.what());
}
CTL_INF("Finished!");
});
auto on_finished = [](const DownloadTask& finishedTask) {
// try to unzip the downloaded file
CTL_INF(
"Engine zip path: " << finishedTask.items[0].localPath.string());

std::filesystem::path extract_path =
finishedTask.items[0].localPath.parent_path().parent_path();

archive_utils::ExtractArchive(
finishedTask.items[0].localPath.string(), extract_path.string());

// remove the downloaded file
try {
std::filesystem::remove(finishedTask.items[0].localPath);
} catch (const std::exception& e) {
CTL_WRN("Could not delete file: " << e.what());
}
CTL_INF("Finished!");
};
if (async) {
auto res = download_service_->AddTask(downloadTask, on_finished);
if (res.has_error()) {
return cpp::fail(res.error());
}
return true;
} else {
return download_service_->AddDownloadTask(downloadTask, on_finished);
}
}
}
return true;
Expand All @@ -350,7 +374,7 @@ cpp::result<bool, std::string> EngineService::DownloadEngine(
}

cpp::result<bool, std::string> EngineService::DownloadCuda(
const std::string& engine) {
const std::string& engine, bool async) {
if (hw_inf_.sys_inf->os == "mac" || engine == kOnnxRepo ||
engine == kOnnxEngine) {
// mac and onnx engine does not require cuda toolkit
Expand Down Expand Up @@ -403,19 +427,27 @@ cpp::result<bool, std::string> EngineService::DownloadCuda(
.localPath = cuda_toolkit_local_path}},
}};

return download_service_->AddDownloadTask(
downloadCudaToolkitTask, [&](const DownloadTask& finishedTask) {
auto engine_path =
file_manager_utils::GetEnginesContainerPath() / engine;
archive_utils::ExtractArchive(finishedTask.items[0].localPath.string(),
engine_path.string());

try {
std::filesystem::remove(finishedTask.items[0].localPath);
} catch (std::exception& e) {
CTL_ERR("Error removing downloaded file: " << e.what());
}
});
auto on_finished = [engine](const DownloadTask& finishedTask) {
auto engine_path = file_manager_utils::GetEnginesContainerPath() / engine;
archive_utils::ExtractArchive(finishedTask.items[0].localPath.string(),
engine_path.string());

try {
std::filesystem::remove(finishedTask.items[0].localPath);
} catch (std::exception& e) {
CTL_ERR("Error removing downloaded file: " << e.what());
}
};
if (async) {
auto res = download_service_->AddTask(downloadCudaToolkitTask, on_finished);
if (res.has_error()) {
return cpp::fail(res.error());
}
return true;
} else {
return download_service_->AddDownloadTask(downloadCudaToolkitTask,
on_finished);
}
}

std::string EngineService::GetMatchedVariant(
Expand Down
8 changes: 6 additions & 2 deletions engine/services/engine_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ class EngineService {
const std::string& engine, const std::string& version = "latest",
const std::string& src = "");

cpp::result<bool, std::string> InstallEngineAsync(
const std::string& engine, const std::string& version = "latest",
const std::string& src = "");

cpp::result<bool, std::string> UninstallEngine(const std::string& engine);

private:
Expand All @@ -55,9 +59,9 @@ class EngineService {
const std::string& path);

cpp::result<bool, std::string> DownloadEngine(
const std::string& engine, const std::string& version = "latest");
const std::string& engine, const std::string& version = "latest", bool async = false);

cpp::result<bool, std::string> DownloadCuda(const std::string& engine);
cpp::result<bool, std::string> DownloadCuda(const std::string& engine, bool async = false);

std::string GetMatchedVariant(const std::string& engine,
const std::vector<std::string>& variants);
Expand Down
18 changes: 8 additions & 10 deletions engine/utils/logging_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,26 @@
// if not verbose only log result to console
inline bool log_verbose = false;

// CLI and Server are sharing logging utils, only print to message if executable is CLI
inline bool is_server = false;

// Only use trantor log
#define CTL_DBG(msg) \
LOG_DEBUG << msg;

#define CTL_DBG(msg) LOG_DEBUG << msg;

#define CTL_INF(msg) \
LOG_INFO << msg;
#define CTL_INF(msg) LOG_INFO << msg;

#define CTL_WRN(msg) \
LOG_WARN << msg;
#define CTL_WRN(msg) LOG_WARN << msg;

// Use std::cout if not verbose, use trantor log if verbose
#define CTL_ERR(msg) LOG_ERROR << msg;

#define CLI_LOG(msg) \
if (log_verbose) { \
if (log_verbose || is_server) { \
LOG_INFO << msg; \
} else { \
std::cout << msg << std::endl; \
}
#define CLI_LOG_ERROR(msg) \
if (log_verbose) { \
if (log_verbose || is_server) { \
LOG_INFO << msg; \
} else { \
LOG_ERROR << msg; \
Expand Down
Loading