fix downloading engines with progress bar

Author: namchuai

engine/cli/commands/engine_get_cmd.cc

@@ -35,7 +35,14 @@ void EngineGetCmd::Exec(const std::string& host, int port,
   };
   auto result = curl_utils::SimpleGetJson(url.ToFullPath());
   if (result.has_error()) {
-    CTL_ERR(result.error());
+    // TODO: refactor this
+    Json::Value root;
+    Json::Reader reader;
+    if (!reader.parse(result.error(), root)) {
+      CLI_LOG(result.error());
+      return;
+    }
+    CLI_LOG(root["message"].asString());
     return;
   }
 

engine/cli/commands/engine_install_cmd.cc

@@ -9,6 +9,14 @@ namespace commands {
 bool EngineInstallCmd::Exec(const std::string& engine,
                             const std::string& version,
                             const std::string& src) {
+  // Start server if server is not started yet
+  if (!commands::IsServerAlive(host_, port_)) {
+    CLI_LOG("Starting server ...");
+    commands::ServerStartCmd ssc;
+    if (!ssc.Exec(host_, port_)) {
+      return false;
+    }
+  }
   // Handle local install, if fails, fallback to remote install
   if (!src.empty()) {
     auto res = engine_service_.UnzipEngine(engine, version, src);
@@ -23,20 +31,12 @@ bool EngineInstallCmd::Exec(const std::string& engine,
   }
 
   if (show_menu_) {
-    // Start server if server is not started yet
-    if (!commands::IsServerAlive(host_, port_)) {
-      CLI_LOG("Starting server ...");
-      commands::ServerStartCmd ssc;
-      if (!ssc.Exec(host_, port_)) {
-        return false;
-      }
-    }
-
     DownloadProgress dp;
     dp.Connect(host_, port_);
     // engine can be small, so need to start ws first
-    auto dp_res = std::async(std::launch::deferred,
-                             [&dp, &engine] { return dp.Handle(engine); });
+    auto dp_res = std::async(std::launch::deferred, [&dp, &engine] {
+      return dp.Handle(DownloadType::Engine);
+    });
     CLI_LOG("Validating download items, please wait..")
 
     auto versions_url = url_parser::Url{
@@ -118,7 +118,7 @@ bool EngineInstallCmd::Exec(const std::string& engine,
 
     bool check_cuda_download = !system_info_utils::GetCudaVersion().empty();
     if (check_cuda_download) {
-      if (!dp.Handle("cuda"))
+      if (!dp.Handle(DownloadType::CudaToolkit))
         return false;
     }
 
@@ -130,10 +130,8 @@ bool EngineInstallCmd::Exec(const std::string& engine,
   DownloadProgress dp;
   dp.Connect(host_, port_);
   // engine can be small, so need to start ws first
-  auto dp_res = std::async(std::launch::deferred, [&dp] {
-    return dp.Handle(DownloadType::Engine);
-  });
-  CLI_LOG("Validating download items, please wait..")
+  auto dp_res = std::async(std::launch::deferred,
+                           [&dp] { return dp.Handle(DownloadType::Engine); });
 
   auto install_url = url_parser::Url{
       .protocol = "http",
@@ -146,12 +144,25 @@ bool EngineInstallCmd::Exec(const std::string& engine,
           },
   };
 
+  if (!version.empty()) {
+    install_url.queries = {{"version", version}};
+  }
+
   auto response = curl_utils::SimplePostJson(install_url.ToFullPath());
   if (response.has_error()) {
-    CTL_ERR(response.error());
+    // TODO: namh refactor later
+    Json::Value root;
+    Json::Reader reader;
+    if (!reader.parse(response.error(), root)) {
+      CLI_LOG(response.error());
+      return false;
+    }
+    CLI_LOG(root["message"].asString());
     return false;
   }
 
+  CLI_LOG("Validating download items, please wait..")
+
   if (!dp_res.get())
     return false;
 

engine/cli/commands/engine_release_cmd.cc

@@ -23,8 +23,9 @@ bool EngineUpdateCmd::Exec(const std::string& host, int port,
   DownloadProgress dp;
   dp.Connect(host, port);
   // engine can be small, so need to start ws first
-  auto dp_res = std::async(std::launch::deferred,
-                           [&dp, &engine] { return dp.Handle(engine); });
+  auto dp_res = std::async(std::launch::deferred, [&dp, &engine] {
+    return dp.Handle(DownloadType::Engine);
+  });
   CLI_LOG("Validating download items, please wait..")
 
   auto update_url = url_parser::Url{
@@ -43,7 +44,7 @@ bool EngineUpdateCmd::Exec(const std::string& host, int port,
 
   bool check_cuda_download = !system_info_utils::GetCudaVersion().empty();
   if (check_cuda_download) {
-    if (!dp.Handle("cuda"))
+    if (!dp.Handle(DownloadType::CudaToolkit))
       return false;
   }
 

engine/cli/commands/engine_release_cmd.h

@@ -29,8 +29,11 @@ void Engines::ListEngine(
   for (const auto& engine : supported_engines) {
     auto installed_engines =
         engine_service_->GetInstalledEngineVariants(engine);
+    if (installed_engines.has_error()) {
+      continue;
+    }
     Json::Value variants(Json::arrayValue);
-    for (const auto& variant : installed_engines) {
+    for (const auto& variant : installed_engines.value()) {
       variants.append(variant.ToJson());
     }
     ret[engine] = variants;
@@ -157,8 +160,16 @@ void Engines::GetInstalledEngineVariants(
     std::function<void(const HttpResponsePtr&)>&& callback,
     const std::string& engine) const {
   auto result = engine_service_->GetInstalledEngineVariants(engine);
+  if (result.has_error()) {
+    Json::Value res;
+    res["message"] = result.error();
+    auto resp = cortex_utils::CreateCortexHttpJsonResponse(res);
+    resp->setStatusCode(k400BadRequest);
+    callback(resp);
+    return;
+  }
   Json::Value releases(Json::arrayValue);
-  for (const auto& variant : result) {
+  for (const auto& variant : result.value()) {
     releases.append(variant.ToJson());
   }
   auto resp = cortex_utils::CreateCortexHttpJsonResponse(releases);

engine/cli/commands/engine_update_cmd.cc

@@ -1,6 +1,5 @@
 import pytest
 import requests
-from test_runner import popen, run
 from test_runner import start_server, stop_server
 
 

engine/controllers/engines.cc

@@ -1,11 +1,11 @@
 import platform
 
 import pytest
-from test_runner import run
-from test_runner import start_server, stop_server
+from test_runner import run, start_server, stop_server
+
 
 class TestCliEngineGet:
-    
+
     @pytest.fixture(autouse=True)
     def setup_and_teardown(self):
         # Setup
@@ -20,9 +20,7 @@ def setup_and_teardown(self):
 
     @pytest.mark.skipif(platform.system() != "Windows", reason="Windows-specific test")
     def test_engines_get_tensorrt_llm_should_not_be_incompatible(self):
-        exit_code, output, error = run(
-            "Get engine", ["engines", "get", "tensorrt-llm"]
-        )
+        exit_code, output, error = run("Get engine", ["engines", "get", "tensorrt-llm"])
         assert exit_code == 0, f"Get engine failed with error: {error}"
         assert (
             "Incompatible" not in output
@@ -37,29 +35,27 @@ def test_engines_get_onnx_should_not_be_incompatible(self):
         ), "onnxruntime should be Ready or Not Installed on Windows"
 
     def test_engines_get_llamacpp_should_not_be_incompatible(self):
-        exit_code, output, error = run(
-            "Get engine", ["engines", "get", "llama-cpp"]
-        )
+        exit_code, output, error = run("Get engine", ["engines", "get", "llama-cpp"])
         assert exit_code == 0, f"Get engine failed with error: {error}"
         assert (
             "Incompatible" not in output
         ), "llama-cpp should be compatible for Windows, MacOs and Linux"
 
     @pytest.mark.skipif(platform.system() != "Darwin", reason="macOS-specific test")
     def test_engines_get_tensorrt_llm_should_be_incompatible_on_macos(self):
-        exit_code, output, error = run(
-            "Get engine", ["engines", "get", "tensorrt-llm"]
-        )
+        exit_code, output, error = run("Get engine", ["engines", "get", "tensorrt-llm"])
         assert exit_code == 0, f"Get engine failed with error: {error}"
         assert (
-            "Incompatible" in output
+            "is not supported on" in output
         ), "tensorrt-llm should be Incompatible on MacOS"
 
     @pytest.mark.skipif(platform.system() != "Darwin", reason="macOS-specific test")
     def test_engines_get_onnx_should_be_incompatible_on_macos(self):
         exit_code, output, error = run("Get engine", ["engines", "get", "onnxruntime"])
         assert exit_code == 0, f"Get engine failed with error: {error}"
-        assert "Incompatible" in output, "onnxruntime should be Incompatible on MacOS"
+        assert (
+            "is not supported on" in output
+        ), "onnxruntime should be Incompatible on MacOS"
 
     @pytest.mark.skipif(platform.system() != "Linux", reason="Linux-specific test")
     def test_engines_get_onnx_should_be_incompatible_on_linux(self):