Review update
diff --git a/extensions/python/PythonDependencyInstaller.cpp b/extensions/python/PythonDependencyInstaller.cpp
index 619f78d..520b7f8 100644
--- a/extensions/python/PythonDependencyInstaller.cpp
+++ b/extensions/python/PythonDependencyInstaller.cpp
@@ -28,14 +28,14 @@
std::string getPythonBinary(const std::shared_ptr<Configure> &configuration) {
#if WIN32
- std::string python_binary = "python";
+ std::string python_binary_ = "python";
#else
- std::string python_binary = "python3";
+ std::string python_binary_ = "python3";
#endif
if (auto binary = configuration->get(minifi::Configuration::nifi_python_env_setup_binary)) {
- python_binary = *binary;
+ python_binary_ = *binary;
}
- return python_binary;
+ return python_binary_;
}
// On Windows when calling a system command using std::system, the whole command needs to be encapsulated in additional quotes,
@@ -51,19 +51,19 @@
} // namespace
PythonDependencyInstaller::PythonDependencyInstaller(const std::shared_ptr<Configure> &configuration) {
- config_state_.python_binary = getPythonBinary(configuration);
+ python_binary_ = getPythonBinary(configuration);
std::string automatic_install_str;
- config_state_.install_python_packages_automatically =
+ install_python_packages_automatically_ =
configuration->get(Configuration::nifi_python_install_packages_automatically, automatic_install_str) && utils::string::toBool(automatic_install_str).value_or(false);
if (auto path = configuration->get(minifi::Configuration::nifi_python_virtualenv_directory)) {
- config_state_.virtualenv_path = *path;
- logger_->log_debug("Python virtualenv path was specified at: {}", config_state_.virtualenv_path.string());
+ virtualenv_path_ = *path;
+ logger_->log_debug("Python virtualenv path was specified at: {}", virtualenv_path_.string());
} else {
logger_->log_debug("No valid python virtualenv path was specified");
}
if (auto python_processor_dir = configuration->get(minifi::Configuration::nifi_python_processor_dir)) {
- config_state_.python_processor_dir = *python_processor_dir;
- logger_->log_debug("Python processor dir was specified at: {}", config_state_.python_processor_dir.string());
+ python_processor_dir_ = *python_processor_dir;
+ logger_->log_debug("Python processor dir was specified at: {}", python_processor_dir_.string());
} else {
logger_->log_debug("No valid python processor dir was not specified in properties");
}
@@ -72,11 +72,11 @@
}
std::vector<std::filesystem::path> PythonDependencyInstaller::getRequirementsFilePaths() const {
- if (!std::filesystem::exists(config_state_.python_processor_dir)) {
+ if (!std::filesystem::exists(python_processor_dir_)) {
return {};
}
std::vector<std::filesystem::path> paths;
- for (const auto& entry : std::filesystem::recursive_directory_iterator(std::filesystem::path{config_state_.python_processor_dir})) {
+ for (const auto& entry : std::filesystem::recursive_directory_iterator(std::filesystem::path{python_processor_dir_})) {
if (std::filesystem::is_regular_file(entry.path()) && entry.path().filename() == "requirements.txt") {
paths.push_back(entry.path());
}
@@ -85,12 +85,12 @@
}
void PythonDependencyInstaller::createVirtualEnvIfSpecified() const {
- if (config_state_.virtualenv_path.empty()) {
+ if (virtualenv_path_.empty()) {
return;
}
- if (!std::filesystem::exists(config_state_.virtualenv_path) || std::filesystem::is_empty(config_state_.virtualenv_path)) {
- logger_->log_info("Creating python virtual env at: {}", config_state_.virtualenv_path.string());
- auto venv_command = "\"" + config_state_.python_binary + "\" -m venv \"" + config_state_.virtualenv_path.string() + "\"";
+ 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));
@@ -99,8 +99,7 @@
}
void PythonDependencyInstaller::installDependenciesFromRequirementsFiles() const {
- std::string automatic_install_str;
- if (!config_state_.isPackageInstallationNeeded()) {
+ if (!isPackageInstallationNeeded()) {
return;
}
auto requirement_file_paths = getRequirementsFilePaths();
@@ -108,11 +107,11 @@
logger_->log_info("Installing python packages from the following requirements.txt file: {}", requirements_file_path.string());
std::string pip_command;
#if WIN32
- pip_command.append("\"").append((config_state_.virtualenv_path / "Scripts" / "activate.bat").string()).append("\" && ");
+ pip_command.append("\"").append((virtualenv_path_ / "Scripts" / "activate.bat").string()).append("\" && ");
#else
- pip_command.append(". \"").append((config_state_.virtualenv_path / "bin" / "activate").string()).append("\" && ");
+ pip_command.append(". \"").append((virtualenv_path_ / "bin" / "activate").string()).append("\" && ");
#endif
- pip_command.append("\"").append(config_state_.python_binary).append("\" -m pip install --no-cache-dir -r \"").append(requirements_file_path.string()).append("\"");
+ pip_command.append("\"").append(python_binary_).append("\" -m pip install --no-cache-dir -r \"").append(requirements_file_path.string()).append("\"");
auto return_value = std::system(encapsulateCommandInQuotesIfNeeded(pip_command).c_str());
if (return_value != 0) {
throw PythonScriptException(fmt::format("The following command to install python packages failed: '{}'", pip_command));
@@ -122,7 +121,7 @@
void PythonDependencyInstaller::evalScript(std::string_view script) {
GlobalInterpreterLock gil;
- const auto script_file = "# -*- coding: utf-8 -*-\n" + std::string(script);
+ const auto script_file = minifi::utils::string::join_pack("# -*- coding: utf-8 -*-\n", script);
auto compiled_string = OwnedObject(Py_CompileString(script_file.c_str(), "<string>", Py_file_input));
if (!compiled_string.get()) {
throw PyException();
@@ -136,16 +135,16 @@
}
void PythonDependencyInstaller::addVirtualenvToPath() const {
- if (config_state_.virtualenv_path.empty()) {
+ if (virtualenv_path_.empty()) {
return;
}
Interpreter::getInterpreter();
- if (!config_state_.virtualenv_path.empty()) {
+ if (!virtualenv_path_.empty()) {
#if WIN32
- std::filesystem::path site_package_path = config_state_.virtualenv_path / "Lib" / "site-packages";
+ std::filesystem::path site_package_path = virtualenv_path_ / "Lib" / "site-packages";
#else
std::string python_dir_name;
- auto lib_path = config_state_.virtualenv_path / "lib";
+ auto lib_path = virtualenv_path_ / "lib";
for (auto const& dir_entry : std::filesystem::directory_iterator{lib_path}) {
if (minifi::utils::string::startsWith(dir_entry.path().filename().string(), "python")) {
python_dir_name = dir_entry.path().filename().string();
@@ -155,7 +154,7 @@
if (python_dir_name.empty()) {
throw PythonScriptException("Could not find python directory under virtualenv lib dir: " + lib_path.string());
}
- std::filesystem::path site_package_path = config_state_.virtualenv_path / "lib" / python_dir_name / "site-packages";
+ std::filesystem::path site_package_path = virtualenv_path_ / "lib" / python_dir_name / "site-packages";
#endif
if (!std::filesystem::exists(site_package_path)) {
throw PythonScriptException("Could not find python site package path: " + site_package_path.string());
diff --git a/extensions/python/PythonDependencyInstaller.h b/extensions/python/PythonDependencyInstaller.h
index 0d9031f..7cb2711 100644
--- a/extensions/python/PythonDependencyInstaller.h
+++ b/extensions/python/PythonDependencyInstaller.h
@@ -20,7 +20,6 @@
#include <memory>
#include <filesystem>
-#include "PythonConfigState.h"
#include "core/logging/Logger.h"
#include "core/logging/LoggerConfiguration.h"
#include "properties/Configure.h"
@@ -37,8 +36,14 @@
void createVirtualEnvIfSpecified() const;
static void evalScript(std::string_view script);
void addVirtualenvToPath() const;
+ bool isPackageInstallationNeeded() const {
+ return install_python_packages_automatically_ && !virtualenv_path_.empty();
+ }
- PythonConfigState config_state_;
+ std::filesystem::path virtualenv_path_;
+ std::filesystem::path python_processor_dir_;
+ std::string python_binary_;
+ bool install_python_packages_automatically_ = false;
std::shared_ptr<core::logging::Logger> logger_ = core::logging::LoggerFactory<PythonDependencyInstaller>::getLogger();
};
diff --git a/extensions/python/PythonScriptEngine.cpp b/extensions/python/PythonScriptEngine.cpp
index 3e40e5b..568f5f9 100644
--- a/extensions/python/PythonScriptEngine.cpp
+++ b/extensions/python/PythonScriptEngine.cpp
@@ -115,7 +115,7 @@
}
void PythonScriptEngine::evalInternal(std::string_view script) {
- const auto script_file = "# -*- coding: utf-8 -*-\n" + std::string(script);
+ const auto script_file = minifi::utils::string::join_pack("# -*- coding: utf-8 -*-\n", script);
auto compiled_string = OwnedObject(Py_CompileString(script_file.c_str(), "<string>", Py_file_input));
if (!compiled_string.get()) {
throw PyException();
diff --git a/extensions/python/PythonScriptEngine.h b/extensions/python/PythonScriptEngine.h
index 5830865..a25edf4 100644
--- a/extensions/python/PythonScriptEngine.h
+++ b/extensions/python/PythonScriptEngine.h
@@ -48,8 +48,6 @@
PythonScriptEngine& operator=(const PythonScriptEngine& other) = delete;
PythonScriptEngine& operator=(PythonScriptEngine&& other) = delete;
- static void initialize(const std::shared_ptr<Configure> &configuration, const std::shared_ptr<core::logging::Logger>& logger);
-
void eval(const std::string& script);
void evalFile(const std::filesystem::path& file_name);
