lordgamez commented on code in PR #2008:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2008#discussion_r2290759481


##########
extensions/python/PythonDependencyInstaller.cpp:
##########
@@ -93,25 +134,30 @@ void 
PythonDependencyInstaller::createVirtualEnvIfSpecified() const {
   }
   if (!std::filesystem::exists(virtualenv_path_) || 
std::filesystem::is_empty(virtualenv_path_)) {
     logger_->log_info("Creating python virtual env at: {}", 
virtualenv_path_.string());
-    auto venv_command = "\"" + python_binary_ + "\" -m venv \"" + 
virtualenv_path_.string() + "\"";
-    auto return_value = 
std::system(encapsulateCommandInQuotesIfNeeded(venv_command).c_str());
-    if (return_value != 0) {
-      throw PythonScriptException(fmt::format("The following command creating 
python virtual env failed: '{}'", venv_command));
+    auto venv_command = "\"" + python_binary_ + "\" -m venv \"" + 
virtualenv_path_.string() + "\" 2>&1";
+    auto result = executeProcess(venv_command);
+    if (result.exit_code != 0) {
+      logger_->log_error("The following command creating python virtual env 
failed: '{}'\nSetup process output:\n{}", venv_command, result.output);
+      throw PythonScriptException(fmt::format("The following command creating 
python virtual env failed: '{}'\nSetup process output:\n{}", venv_command, 
result.output));
     }
   }
 }
 
 void PythonDependencyInstaller::runInstallCommandInVirtualenv(const 
std::string& install_command) const {
   std::string command_with_virtualenv;
 #if WIN32
-  command_with_virtualenv.append("\"").append((virtualenv_path_ / "Scripts" / 
"activate.bat").string()).append("\" && ");
+  command_with_virtualenv.append("\"").append((virtualenv_path_ / "Scripts" / 
"activate.bat").string()).append("\" 2>&1 && ");
 #else
-  command_with_virtualenv.append(". \"").append((virtualenv_path_ / "bin" / 
"activate").string()).append("\" && ");
+  command_with_virtualenv.append(". \"").append((virtualenv_path_ / "bin" / 
"activate").string()).append("\" 2>&1 && ");
 #endif
   command_with_virtualenv.append(install_command);
-  auto return_value = 
std::system(encapsulateCommandInQuotesIfNeeded(command_with_virtualenv).c_str());
-  if (return_value != 0) {
-    throw PythonScriptException(fmt::format("The following command to install 
python packages failed: '{}'", command_with_virtualenv));
+
+  auto result = executeProcess(command_with_virtualenv + " 2>&1");

Review Comment:
   Updated in 
https://github.com/apache/nifi-minifi-cpp/pull/2008/commits/161a1694bbf84b37597e83031bd8e5710fb5d043



##########
extensions/python/PythonDependencyInstaller.cpp:
##########
@@ -93,25 +134,30 @@ void 
PythonDependencyInstaller::createVirtualEnvIfSpecified() const {
   }
   if (!std::filesystem::exists(virtualenv_path_) || 
std::filesystem::is_empty(virtualenv_path_)) {
     logger_->log_info("Creating python virtual env at: {}", 
virtualenv_path_.string());
-    auto venv_command = "\"" + python_binary_ + "\" -m venv \"" + 
virtualenv_path_.string() + "\"";
-    auto return_value = 
std::system(encapsulateCommandInQuotesIfNeeded(venv_command).c_str());
-    if (return_value != 0) {
-      throw PythonScriptException(fmt::format("The following command creating 
python virtual env failed: '{}'", venv_command));
+    auto venv_command = "\"" + python_binary_ + "\" -m venv \"" + 
virtualenv_path_.string() + "\" 2>&1";
+    auto result = executeProcess(venv_command);
+    if (result.exit_code != 0) {
+      logger_->log_error("The following command creating python virtual env 
failed: '{}'\nSetup process output:\n{}", venv_command, result.output);
+      throw PythonScriptException(fmt::format("The following command creating 
python virtual env failed: '{}'\nSetup process output:\n{}", venv_command, 
result.output));
     }
   }
 }
 
 void PythonDependencyInstaller::runInstallCommandInVirtualenv(const 
std::string& install_command) const {
   std::string command_with_virtualenv;
 #if WIN32
-  command_with_virtualenv.append("\"").append((virtualenv_path_ / "Scripts" / 
"activate.bat").string()).append("\" && ");
+  command_with_virtualenv.append("\"").append((virtualenv_path_ / "Scripts" / 
"activate.bat").string()).append("\" 2>&1 && ");
 #else
-  command_with_virtualenv.append(". \"").append((virtualenv_path_ / "bin" / 
"activate").string()).append("\" && ");
+  command_with_virtualenv.append(". \"").append((virtualenv_path_ / "bin" / 
"activate").string()).append("\" 2>&1 && ");
 #endif
   command_with_virtualenv.append(install_command);
-  auto return_value = 
std::system(encapsulateCommandInQuotesIfNeeded(command_with_virtualenv).c_str());
-  if (return_value != 0) {
-    throw PythonScriptException(fmt::format("The following command to install 
python packages failed: '{}'", command_with_virtualenv));
+
+  auto result = executeProcess(command_with_virtualenv + " 2>&1");
+  if (result.exit_code != 0) {
+    logger_->log_error("Failed to install python packages to virtualenv. 
Install process output:\n{}", result.output);
+    throw PythonScriptException(fmt::format("Failed to install python packages 
to virtualenv. Install process output:\n{}", result.output));

Review Comment:
   Updated in 
https://github.com/apache/nifi-minifi-cpp/pull/2008/commits/161a1694bbf84b37597e83031bd8e5710fb5d043



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to