[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444195397
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,220 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+#include "utils/TestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::createTempDir;
+using org::apache::nifi::minifi::utils::putFileToDir;
+using org::apache::nifi::minifi::utils::createTempDirWithFile;
+using org::apache::nifi::minifi::utils::getFileContent;
+
+class ExecutePythonProcessorTestBase {
+ public:
+ ExecutePythonProcessorTestBase() :
+logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+reInitialize();
+ }
+ virtual ~ExecutePythonProcessorTestBase() {
+logTestController_.reset();
+logTestController_.setDebug();
+
logTestController_.setDebug();
+logTestController_.setDebug();
+logTestController_.setDebug();
+ }
+
+ protected:
+ void reInitialize() {
+testController_.reset(new TestController());
+plan_ = testController_->createPlan();
+ }
+
+ std::string getScriptFullPath(const std::string& script_file_name) {
+return SCRIPT_FILES_DIRECTORY + utils::file::FileUtils::get_separator() +
script_file_name;
+ }
+
+ static const std::string TEST_FILE_NAME;
+ static const std::string TEST_FILE_CONTENT;
+ static const std::string SCRIPT_FILES_DIRECTORY;
+
+ std::unique_ptr testController_;
+ std::shared_ptr plan_;
+ LogTestController& logTestController_;
+ std::shared_ptr logger_;
+};
+
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_NAME{
"test_file.txt" };
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_CONTENT{ "Test
text\n" };
+const std::string ExecutePythonProcessorTestBase::SCRIPT_FILES_DIRECTORY{
"test_scripts" };
+
+class SimplePythonFlowFileTransferTest : public ExecutePythonProcessorTestBase
{
+ public:
+ enum class Expectation {
+OUTPUT_FILE_MATCHES_INPUT,
+RUNTIME_RELATIONSHIP_EXCEPTION,
+PROCESSOR_INITIALIZATION_EXCEPTION
+ };
+ SimplePythonFlowFileTransferTest() : ExecutePythonProcessorTestBase{} {}
+
+ protected:
+ void testSimpleFilePassthrough(const Expectation expectation, const
core::Relationship& execute_python_out_conn, const std::string&
used_as_script_file, const std::string& used_as_script_body) {
+reInitialize();
+const std::string input_dir = createTempDirWithFile(testController_.get(),
TEST_FILE_NAME, TEST_FILE_CONTENT);
+const std::string output_dir = createTempDir(testController_.get());
+
+addGetFileProcessorToPlan(input_dir);
+if (Expectation::PROCESSOR_INITIALIZATION_EXCEPTION == expectation) {
+ REQUIRE_THROWS(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+ return;
+}
+REQUIRE_NOTHROW(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+addPutFileProcessorToPlan(execute_python_out_conn, output_dir);
+
+plan_->runNextProcessor(); // GetFile
+if (Expectation::RUNTIME_RELATIONSHIP_EXCEPTION == expectation) {
+ REQUIRE_THROWS(plan_->runNextProcessor()); // ExecutePythonProcessor
+ return;
+}
+REQUIRE_NOTHROW(plan_->runNextProcessor()); // ExecutePythonProcessor
+plan_->runNextProcessor(); // PutFile
+
+const std::string output_file_path = output_dir +
utils::file::FileUtils::get_separator() + TEST_FILE_NAME;
+
+if (Expectation::OUTPUT_FILE_MATCHES_INPUT == expectation) {
+ const std::string output_file_content{ getFileContent(output_file_path)
};
+ REQUIRE(TEST_FILE_CONTENT == output_file_content);
+}
+ }
+ void testsStatefulProcessor() {
+reInitialize();
+const std::string output_dir = createTempDir(testController_.get());
+
+auto executePythonProcessor =
plan_->addProcessor("ExecutePythonProcessor",
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444195348
##
File path: extensions/script/python/ExecutePythonProcessor.h
##
@@ -18,12 +18,16 @@
* limitations under the License.
*/
-#ifndef NIFI_MINIFI_CPP_EXECUTEPYPROC_H
-#define NIFI_MINIFI_CPP_EXECUTEPYPROC_H
+#ifndef EXTENSIONS_SCRIPT_PYTHON_EXECUTEPYTHONPROCESSOR_H_
+#define EXTENSIONS_SCRIPT_PYTHON_EXECUTEPYTHONPROCESSOR_H_
-#include
-#include
-#include
+#include
+#include
+#include
+
+#include // NOLINT
+#include // NOLINT
Review comment:
Noted :)
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,220 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+#include "utils/TestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::createTempDir;
+using org::apache::nifi::minifi::utils::putFileToDir;
+using org::apache::nifi::minifi::utils::createTempDirWithFile;
+using org::apache::nifi::minifi::utils::getFileContent;
+
+class ExecutePythonProcessorTestBase {
+ public:
+ ExecutePythonProcessorTestBase() :
+logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+reInitialize();
+ }
+ virtual ~ExecutePythonProcessorTestBase() {
+logTestController_.reset();
+logTestController_.setDebug();
+
logTestController_.setDebug();
+logTestController_.setDebug();
+logTestController_.setDebug();
+ }
+
+ protected:
+ void reInitialize() {
+testController_.reset(new TestController());
+plan_ = testController_->createPlan();
+ }
+
+ std::string getScriptFullPath(const std::string& script_file_name) {
+return SCRIPT_FILES_DIRECTORY + utils::file::FileUtils::get_separator() +
script_file_name;
+ }
+
+ static const std::string TEST_FILE_NAME;
+ static const std::string TEST_FILE_CONTENT;
+ static const std::string SCRIPT_FILES_DIRECTORY;
+
+ std::unique_ptr testController_;
+ std::shared_ptr plan_;
+ LogTestController& logTestController_;
+ std::shared_ptr logger_;
+};
+
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_NAME{
"test_file.txt" };
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_CONTENT{ "Test
text\n" };
+const std::string ExecutePythonProcessorTestBase::SCRIPT_FILES_DIRECTORY{
"test_scripts" };
+
+class SimplePythonFlowFileTransferTest : public ExecutePythonProcessorTestBase
{
+ public:
+ enum class Expectation {
+OUTPUT_FILE_MATCHES_INPUT,
+RUNTIME_RELATIONSHIP_EXCEPTION,
+PROCESSOR_INITIALIZATION_EXCEPTION
+ };
+ SimplePythonFlowFileTransferTest() : ExecutePythonProcessorTestBase{} {}
+
+ protected:
+ void testSimpleFilePassthrough(const Expectation expectation, const
core::Relationship& execute_python_out_conn, const std::string&
used_as_script_file, const std::string& used_as_script_body) {
+reInitialize();
+const std::string input_dir = createTempDirWithFile(testController_.get(),
TEST_FILE_NAME, TEST_FILE_CONTENT);
+const std::string output_dir = createTempDir(testController_.get());
+
+addGetFileProcessorToPlan(input_dir);
+if (Expectation::PROCESSOR_INITIALIZATION_EXCEPTION == expectation) {
+ REQUIRE_THROWS(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+ return;
+}
+REQUIRE_NOTHROW(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+addPutFileProcessorToPlan(execute_python_out_conn, output_dir);
+
+plan_->runNextProcessor(); // GetFile
+if (Expectation::RUNTIME_RELATIONSHIP_EXCEPTION == expectation) {
+ REQUIRE_THROWS(plan_->runNextProcessor()); // ExecutePythonProcessor
+ return;
+}
+REQUIRE_NOTHROW(plan_->runNextProcessor()); // ExecutePythonProcessor
+plan_->runNextProcessor(); // PutFile
+
+const std::string output_file_path = output_dir +
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444194796
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,155 +35,185 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute. Only one of Script File or Script Body
may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute. Only one of Script File or Script Body may be
used)", "");
core::Property ExecutePythonProcessor::ModuleDirectory("Module Directory", //
NOLINT
-R"(Comma-separated list of paths to files and/or directories which
- contain modules required by
the script)", "");
+R"(Comma-separated list of paths to files and/or directories which contain
modules required by the script)", "");
core::Relationship ExecutePythonProcessor::Success("success", "Script
successes"); // NOLINT
core::Relationship ExecutePythonProcessor::Failure("failure", "Script
failures"); // NOLINT
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if (script_to_exec_.size()) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
if (!valid_init_) {
-throw std::runtime_error("Could not correctly in initialize " + getName());
- }
- context->getProperty(ScriptFile.getName(), script_file_);
- context->getProperty(ModuleDirectory.getName(), module_directory_);
- if (script_file_.empty() && script_engine_.empty()) {
-logger_->log_error("Script File must be defined");
-return;
+throw std::runtime_error("Could not correctly initialize " + getName());
}
-
try {
-std::shared_ptr engine;
-
-// Use an existing engine, if one is available
-if (script_engine_q_.try_dequeue(engine)) {
- logger_->log_debug("Using available %s script engine instance",
script_engine_);
-} else {
- logger_->log_info("Creating new %s script instance", script_engine_);
- logger_->log_info("Approximately %d %s script instances created for this
processor", script_engine_q_.size_approx(), script_engine_);
-
- engine = createEngine();
-
- if (engine == nullptr) {
-throw std::runtime_error("No script engine available");
- }
-
- if (!script_file_.empty()) {
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444194985
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,155 +35,184 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute. Only one of Script File or Script Body
may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
Review comment:
Replaced.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444192954
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,67 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include
+
+#include "../../test/TestBase.h"
+#include "utils/file/FileUtils.h"
+#include "utils/Environment.h"
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string createTempDir(TestController* testController) {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ REQUIRE(file::FileUtils::is_directory(temp_dir.c_str()));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + file::FileUtils::get_separator() +
file_name);
Review comment:
As requested.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444192521
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,155 +35,184 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute. Only one of Script File or Script Body
may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute. Only one of Script File or Script Body may be
used)", "");
core::Property ExecutePythonProcessor::ModuleDirectory("Module Directory", //
NOLINT
-R"(Comma-separated list of paths to files and/or directories which
- contain modules required by
the script)", "");
+R"(Comma-separated list of paths to files and/or directories which contain
modules required by the script)", "");
core::Relationship ExecutePythonProcessor::Success("success", "Script
successes"); // NOLINT
core::Relationship ExecutePythonProcessor::Failure("failure", "Script
failures"); // NOLINT
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if (script_to_exec_.size()) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
if (!valid_init_) {
-throw std::runtime_error("Could not correctly in initialize " + getName());
- }
- context->getProperty(ScriptFile.getName(), script_file_);
- context->getProperty(ModuleDirectory.getName(), module_directory_);
- if (script_file_.empty() && script_engine_.empty()) {
-logger_->log_error("Script File must be defined");
-return;
+throw std::runtime_error("Could not correctly initialize " + getName());
}
-
try {
-std::shared_ptr engine;
-
-// Use an existing engine, if one is available
-if (script_engine_q_.try_dequeue(engine)) {
- logger_->log_debug("Using available %s script engine instance",
script_engine_);
-} else {
- logger_->log_info("Creating new %s script instance", script_engine_);
- logger_->log_info("Approximately %d %s script instances created for this
processor", script_engine_q_.size_approx(), script_engine_);
-
- engine = createEngine();
-
- if (engine == nullptr) {
-throw std::runtime_error("No script engine available");
- }
-
- if (!script_file_.empty()) {
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784: URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444186455 ## File path: extensions/script/python/ExecutePythonProcessor.h ## @@ -18,12 +18,16 @@ * limitations under the License. */ -#ifndef NIFI_MINIFI_CPP_EXECUTEPYPROC_H -#define NIFI_MINIFI_CPP_EXECUTEPYPROC_H +#ifndef EXTENSIONS_SCRIPT_PYTHON_EXECUTEPYTHONPROCESSOR_H_ +#define EXTENSIONS_SCRIPT_PYTHON_EXECUTEPYTHONPROCESSOR_H_ -#include -#include -#include +#include +#include +#include + +#include // NOLINT +#include // NOLINT Review comment: I replaced the `<>` with "", but for concurrentqueue, one gets a different error: > 28:1 error cpplint Include the directory when naming .h files [build/include_subdir] [4] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784: URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r444186455 ## File path: extensions/script/python/ExecutePythonProcessor.h ## @@ -18,12 +18,16 @@ * limitations under the License. */ -#ifndef NIFI_MINIFI_CPP_EXECUTEPYPROC_H -#define NIFI_MINIFI_CPP_EXECUTEPYPROC_H +#ifndef EXTENSIONS_SCRIPT_PYTHON_EXECUTEPYTHONPROCESSOR_H_ +#define EXTENSIONS_SCRIPT_PYTHON_EXECUTEPYTHONPROCESSOR_H_ -#include -#include -#include +#include +#include +#include + +#include // NOLINT +#include // NOLINT Review comment: I replaced the `<>` with "", but for `concurrentqueue.h`, one gets a different error: > 28:1 error cpplint Include the directory when naming .h files [build/include_subdir] [4] 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435881848
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+std::string createTempDir(TestController* testController) {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator() +
file_name);
+ std::ofstream out_file(file_path);
Review comment:
Updated.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435878350
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+std::string createTempDir(TestController* testController) {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator() +
file_name);
+ std::ofstream out_file(file_path);
+ if (out_file.is_open()) {
+out_file << content;
+ }
+ return file_path;
+}
+
+std::string createTempDirWithFile(TestController* testController, const
std::string& file_name, const std::string& content) {
+ std::string temp_dir = createTempDir(testController);
+ putFileToDir(temp_dir, file_name, content);
+ return temp_dir;
Review comment:
I think this was exactly what I needed: the name of directory with a
single file in it, in order to have the processor `GetFile` exactly one target
to run on. Maybe I should rename the function to indicate that this is giving
back the path of a temp dir with a file, or add two implementations
`getFilePathForNewTempDirWithSingleFile` and
`getTempDirPathForNewTempDirWithSingleFile`?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435878350
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+std::string createTempDir(TestController* testController) {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator() +
file_name);
+ std::ofstream out_file(file_path);
+ if (out_file.is_open()) {
+out_file << content;
+ }
+ return file_path;
+}
+
+std::string createTempDirWithFile(TestController* testController, const
std::string& file_name, const std::string& content) {
+ std::string temp_dir = createTempDir(testController);
+ putFileToDir(temp_dir, file_name, content);
+ return temp_dir;
Review comment:
I think this was exactly what I needed: the name of directory with a
single file in it, in order to have the processor `GetFile` exactly one target
to run on. Maybe I should rename the function to indicate that this is giving
back the path of a temp dir with a file?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435867932
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,155 +35,185 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute. Only one of Script File or Script Body
may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute. Only one of Script File or Script Body may be
used)", "");
core::Property ExecutePythonProcessor::ModuleDirectory("Module Directory", //
NOLINT
-R"(Comma-separated list of paths to files and/or directories which
- contain modules required by
the script)", "");
+R"(Comma-separated list of paths to files and/or directories which contain
modules required by the script)", "");
core::Relationship ExecutePythonProcessor::Success("success", "Script
successes"); // NOLINT
core::Relationship ExecutePythonProcessor::Failure("failure", "Script
failures"); // NOLINT
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if (script_to_exec_.size()) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
if (!valid_init_) {
-throw std::runtime_error("Could not correctly in initialize " + getName());
- }
- context->getProperty(ScriptFile.getName(), script_file_);
- context->getProperty(ModuleDirectory.getName(), module_directory_);
- if (script_file_.empty() && script_engine_.empty()) {
-logger_->log_error("Script File must be defined");
-return;
+throw std::runtime_error("Could not correctly initialize " + getName());
}
-
try {
-std::shared_ptr engine;
-
-// Use an existing engine, if one is available
-if (script_engine_q_.try_dequeue(engine)) {
- logger_->log_debug("Using available %s script engine instance",
script_engine_);
-} else {
- logger_->log_info("Creating new %s script instance", script_engine_);
- logger_->log_info("Approximately %d %s script instances created for this
processor", script_engine_q_.size_approx(), script_engine_);
-
- engine = createEngine();
-
- if (engine == nullptr) {
-throw std::runtime_error("No script engine available");
- }
-
- if (!script_file_.empty()) {
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435717734
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,155 +35,188 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute. Only one of Script File or Script Body
may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute. Only one of Script File or Script Body may be
used)", "");
core::Property ExecutePythonProcessor::ModuleDirectory("Module Directory", //
NOLINT
-R"(Comma-separated list of paths to files and/or directories which
- contain modules required by
the script)", "");
+R"(Comma-separated list of paths to files and/or directories which contain
modules required by the script)", "");
core::Relationship ExecutePythonProcessor::Success("success", "Script
successes"); // NOLINT
core::Relationship ExecutePythonProcessor::Failure("failure", "Script
failures"); // NOLINT
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if (script_to_exec_.size()) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
if (!valid_init_) {
-throw std::runtime_error("Could not correctly in initialize " + getName());
- }
- context->getProperty(ScriptFile.getName(), script_file_);
- context->getProperty(ModuleDirectory.getName(), module_directory_);
- if (script_file_.empty() && script_engine_.empty()) {
-logger_->log_error("Script File must be defined");
-return;
+throw std::runtime_error("Could not correctly initialize " + getName());
}
-
try {
-std::shared_ptr engine;
-
-// Use an existing engine, if one is available
-if (script_engine_q_.try_dequeue(engine)) {
- logger_->log_debug("Using available %s script engine instance",
script_engine_);
-} else {
- logger_->log_info("Creating new %s script instance", script_engine_);
- logger_->log_info("Approximately %d %s script instances created for this
processor", script_engine_q_.size_approx(), script_engine_);
-
- engine = createEngine();
-
- if (engine == nullptr) {
-throw std::runtime_error("No script engine available");
- }
-
- if (!script_file_.empty()) {
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435717207
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,220 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+#include "utils/TestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::createTempDir;
+using org::apache::nifi::minifi::utils::putFileToDir;
+using org::apache::nifi::minifi::utils::createTempDirWithFile;
+using org::apache::nifi::minifi::utils::getFileContent;
+
+class ExecutePythonProcessorTestBase {
+ public:
+ExecutePythonProcessorTestBase() :
+ logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+ reInitialize();
+}
+virtual ~ExecutePythonProcessorTestBase() {
+ logTestController_.reset();
+ logTestController_.setDebug();
+
logTestController_.setDebug();
+ logTestController_.setDebug();
+ logTestController_.setDebug();
+}
+
+ protected:
+void reInitialize() {
+ testController_.reset(new TestController());
+ plan_ = testController_->createPlan();
+}
+
+std::string getScriptFullPath(const std::string& script_file_name) {
+ return SCRIPT_FILES_DIRECTORY + utils::file::FileUtils::get_separator()
+ script_file_name;
+}
+
+static const std::string TEST_FILE_NAME;
+static const std::string TEST_FILE_CONTENT;
+static const std::string SCRIPT_FILES_DIRECTORY;
+
+std::unique_ptr testController_;
+std::shared_ptr plan_;
+LogTestController& logTestController_;
+std::shared_ptr logger_;
+};
+
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_NAME{
"test_file.txt" };
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_CONTENT{ "Test
text\n" };
+const std::string ExecutePythonProcessorTestBase::SCRIPT_FILES_DIRECTORY{
"test_scripts" };
+
+class SimplePythonFlowFileTransferTest : public ExecutePythonProcessorTestBase
{
+ public:
+ enum class Expectation {
+OUTPUT_FILE_MATCHES_INPUT,
+RUNTIME_RELATIONSHIP_EXCEPTION,
+PROCESSOR_INITIALIZATION_EXCEPTION
+ };
+ SimplePythonFlowFileTransferTest() : ExecutePythonProcessorTestBase{} {}
+
+ protected:
+ void testSimpleFilePassthrough(const Expectation expectation, const
core::Relationship& execute_python_out_conn, const std::string&
used_as_script_file, const std::string& used_as_script_body) {
+reInitialize();
+const std::string input_dir = createTempDirWithFile(testController_.get(),
TEST_FILE_NAME, TEST_FILE_CONTENT);
+const std::string output_dir = createTempDir(testController_.get());
+
+addGetFileProcessorToPlan(input_dir);
+if (Expectation::PROCESSOR_INITIALIZATION_EXCEPTION == expectation) {
+ REQUIRE_THROWS(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+ return;
+}
+REQUIRE_NOTHROW(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+addPutFileProcessorToPlan(execute_python_out_conn, output_dir);
+
+plan_->runNextProcessor(); // GetFile
+if (Expectation::RUNTIME_RELATIONSHIP_EXCEPTION == expectation) {
+ REQUIRE_THROWS(plan_->runNextProcessor()); // ExecutePythonProcessor
+ return;
+}
+REQUIRE_NOTHROW(plan_->runNextProcessor()); // ExecutePythonProcessor
+plan_->runNextProcessor(); // PutFile
+
+const std::string output_file_path = output_dir +
utils::file::FileUtils::get_separator() + TEST_FILE_NAME;
+
+if (Expectation::OUTPUT_FILE_MATCHES_INPUT == expectation) {
+ const std::string output_file_content{ getFileContent(output_file_path)
};
+ REQUIRE(TEST_FILE_CONTENT == output_file_content);
+}
+ }
+ void testsStatefulProcessor() {
+reInitialize();
+const std::string output_dir = createTempDir(testController_.get());
+
+auto executePythonProcessor =
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435717346
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,220 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+#include "utils/TestUtils.h"
+
+namespace {
+using org::apache::nifi::minifi::utils::createTempDir;
+using org::apache::nifi::minifi::utils::putFileToDir;
+using org::apache::nifi::minifi::utils::createTempDirWithFile;
+using org::apache::nifi::minifi::utils::getFileContent;
+
+class ExecutePythonProcessorTestBase {
+ public:
+ExecutePythonProcessorTestBase() :
+ logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+ reInitialize();
+}
+virtual ~ExecutePythonProcessorTestBase() {
+ logTestController_.reset();
+ logTestController_.setDebug();
+
logTestController_.setDebug();
+ logTestController_.setDebug();
+ logTestController_.setDebug();
+}
+
+ protected:
+void reInitialize() {
+ testController_.reset(new TestController());
+ plan_ = testController_->createPlan();
+}
+
+std::string getScriptFullPath(const std::string& script_file_name) {
+ return SCRIPT_FILES_DIRECTORY + utils::file::FileUtils::get_separator()
+ script_file_name;
+}
+
+static const std::string TEST_FILE_NAME;
+static const std::string TEST_FILE_CONTENT;
+static const std::string SCRIPT_FILES_DIRECTORY;
+
+std::unique_ptr testController_;
+std::shared_ptr plan_;
+LogTestController& logTestController_;
+std::shared_ptr logger_;
+};
+
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_NAME{
"test_file.txt" };
+const std::string ExecutePythonProcessorTestBase::TEST_FILE_CONTENT{ "Test
text\n" };
+const std::string ExecutePythonProcessorTestBase::SCRIPT_FILES_DIRECTORY{
"test_scripts" };
+
+class SimplePythonFlowFileTransferTest : public ExecutePythonProcessorTestBase
{
+ public:
+ enum class Expectation {
+OUTPUT_FILE_MATCHES_INPUT,
+RUNTIME_RELATIONSHIP_EXCEPTION,
+PROCESSOR_INITIALIZATION_EXCEPTION
+ };
+ SimplePythonFlowFileTransferTest() : ExecutePythonProcessorTestBase{} {}
+
+ protected:
+ void testSimpleFilePassthrough(const Expectation expectation, const
core::Relationship& execute_python_out_conn, const std::string&
used_as_script_file, const std::string& used_as_script_body) {
+reInitialize();
+const std::string input_dir = createTempDirWithFile(testController_.get(),
TEST_FILE_NAME, TEST_FILE_CONTENT);
+const std::string output_dir = createTempDir(testController_.get());
+
+addGetFileProcessorToPlan(input_dir);
+if (Expectation::PROCESSOR_INITIALIZATION_EXCEPTION == expectation) {
+ REQUIRE_THROWS(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+ return;
+}
+REQUIRE_NOTHROW(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+addPutFileProcessorToPlan(execute_python_out_conn, output_dir);
+
+plan_->runNextProcessor(); // GetFile
+if (Expectation::RUNTIME_RELATIONSHIP_EXCEPTION == expectation) {
+ REQUIRE_THROWS(plan_->runNextProcessor()); // ExecutePythonProcessor
+ return;
+}
+REQUIRE_NOTHROW(plan_->runNextProcessor()); // ExecutePythonProcessor
+plan_->runNextProcessor(); // PutFile
+
+const std::string output_file_path = output_dir +
utils::file::FileUtils::get_separator() + TEST_FILE_NAME;
+
+if (Expectation::OUTPUT_FILE_MATCHES_INPUT == expectation) {
+ const std::string output_file_content{ getFileContent(output_file_path)
};
+ REQUIRE(TEST_FILE_CONTENT == output_file_content);
+}
+ }
+ void testsStatefulProcessor() {
+reInitialize();
+const std::string output_dir = createTempDir(testController_.get());
+
+auto executePythonProcessor =
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435437798
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
Review comment:
Thanks for the suggestion, I removed the redundant function.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435437263
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
Replaced.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435437097
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -46,144 +50,177 @@ core::Relationship
ExecutePythonProcessor::Failure("failure", "Script failures")
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if ("" != script_to_exec_) {
Review comment:
Due to [performance
impact](https://stackoverflow.com/questions/62081977/why-is-there-no-optimization-for-checking-for-empty-string-via-comparison)
I went with replacing `"" ==` and `"" !=` with `.empty()` and `.size()` calls
respectively.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435431799
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+std::string createTempDir(TestController* testController) {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
Review comment:
Changed to the FileUtils version.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435430949
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+std::string createTempDir(TestController* testController) {
Review comment:
The main issue, is that `createTempDirectory` swallows whatever is
returned from `boost::filesystem::create_directory()`
Relevant code:
```c++
create_dir(tempDirectory); << RETURN VALUE NOT HANDLED
return tempDirectory;
```
From `create_dir()`:
```c++
if(boost::filesystem::create_directory(dir))
{
return 0;
}
else
{
return -1;
}
```
Comparison of the interfaces:
| | FileUtils::create_temp_directory() |
TestController::createTempDirectory() | utils::createTempDir() |
| - | - | - | - |
| Needs format? | Yes | Yes | ✅ NO|
| Checks if dir creation successful? | No | No | ✅ Yes|
| Cleans after itself? | No | ✅ Yes | ✅ Yes |
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435404598
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
Checked if this pattern is recurring:
```bash
find . -name '*.cpp' | xargs grep -Hn -A2 "core::Property" | grep -v '\-\-'
| perl -p -e "s/^(.*?)[:-]([0-9]+).*$/\1 \2/g" | xargs -n2 sh -c 'perl -ne
"print "\""$1:"\"" . \$_ if $. == $2" $1' sh | grep '\"' | less | awk -F \"
'!(NF % 2)'
./extensions/script/ExecuteScript.cpp:
R"(Path to script file to execute.
./extensions/script/ExecuteScript.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/ExecuteScript.cpp:
R"(Body of script to execute.
./extensions/script/ExecuteScript.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/ExecuteScript.cpp:
R"(Comma-separated list of paths to files and/or directories which
./extensions/script/ExecuteScript.cpp:
contain modules required by the script)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Path to script
file to execute.
./extensions/script/python/ExecutePythonProcessor.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Script to
execute.
./extensions/script/python/ExecutePythonProcessor.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Comma-separated
list of paths to files and/or directories which
./extensions/script/python/ExecutePythonProcessor.cpp:
contain modules required by the script)", "");
```
Seems like the only two files having this pattern is `ExecuteScript` and
`ExecutePythonProcessor.cpp`. This is probably due to the boilerplate of
`ExecutePythonProcessor` was originally copied from `ExecuteScript.cpp`. Should
I remove the redundant white spaces from both places?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435404598
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
Checked if this pattern is recurring:
```bash
find . -name '*.cpp' | xargs grep -Hn -A2 "core::Property" | grep -v '\-\-'
| perl -p -e "s/^(.*?)[:-]([0-9]+).*$/\1 \2/g" | xargs -n2 sh -c 'perl -ne
"print "\""$1:"\"" . \$_ if $. == $2" $1' sh | grep '\"' | less | awk -F \"
'!(NF % 2)'
./extensions/script/ExecuteScript.cpp:
R"(Path to script file to execute.
./extensions/script/ExecuteScript.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/ExecuteScript.cpp:
R"(Body of script to execute.
./extensions/script/ExecuteScript.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/ExecuteScript.cpp:
R"(Comma-separated list of paths to files and/or directories which
./extensions/script/ExecuteScript.cpp:
contain modules required by the script)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Path to script
file to execute.
./extensions/script/python/ExecutePythonProcessor.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Script to
execute.
./extensions/script/python/ExecutePythonProcessor.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Comma-separated
list of paths to files and/or directories which
./extensions/script/python/ExecutePythonProcessor.cpp:
contain modules required by the script)", "");
```
Seems like the only two files having this pattern is `ExecuteScript` and
`ExecutePythonProcessor.cpp`. This is probably due to the boilerplate of
`ExecutePythonProcessor` was originally copied from `ExecuteScript.cpp. Should
I remove the redundant white spaces from both places?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435404598
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
Checked if this pattern is recurring:
```bash
find . -name '*.cpp' | xargs grep -Hn -A2 "core::Property" | grep -v '\-\-'
| perl -p -e "s/^(.*?)[:-]([0-9]+).*$/\1 \2/g" | xargs -n2 sh -c 'perl -ne
"print "\""$1:"\"" . \$_ if $. == $2" $1' sh | grep '\"' | less | awk -F \"
'!(NF % 2)'
./extensions/script/ExecuteScript.cpp:
R"(Path to script file to execute.
./extensions/script/ExecuteScript.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/ExecuteScript.cpp:
R"(Body of script to execute.
./extensions/script/ExecuteScript.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/ExecuteScript.cpp:
R"(Comma-separated list of paths to files and/or directories which
./extensions/script/ExecuteScript.cpp:
contain modules required by the script)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Path to script
file to execute.
./extensions/script/python/ExecutePythonProcessor.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Script to
execute.
./extensions/script/python/ExecutePythonProcessor.cpp:
Only one of Script File or Script Body may be used)", "");
./extensions/script/python/ExecutePythonProcessor.cpp:R"(Comma-separated
list of paths to files and/or directories which
./extensions/script/python/ExecutePythonProcessor.cpp:
contain modules required by the script)", "");
```
Seems like the only two files having this pattern is `ExecuteScript` and
`ExecutePythonProcessor.cpp`. This is probably due to the boilerplate of
`ExecutePythonProcessor` was originally copied from `ExecuteScript.cpp. Should
I remove both?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435334768
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
This is the state I restored from the original documentation. I have no
knowledge on why the whitespaces might be useful.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r435333611
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
Review comment:
I cannot find it there:
```bash
doxygen -g && perl -pi -e "s/GENERATE_(HTML|LATEX)(\s*)=.*$/GENERATE_\1\2=
NO/g; s/GENERATE_MAN(\s*)=.*$/GENERATE_MAN\1= YES/g;
s;INPUT(\s*)=.*$;INPUT\1=$(find . -name FileUtils.h);g" Doxyfile && doxygen &&
man ./man/man3/*
```
https://user-images.githubusercontent.com/64011968/83774523-123c6d80-a686-11ea-8ed2-1d1f54e2b8e3.png;>
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r433931716
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
I do not know, they were split by the original design. I was not sure if
this is a hack used for displaying the definition in the EFM, so I left the
split in place.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r433935221
##
File path: libminifi/include/utils/TestUtils.h
##
@@ -0,0 +1,80 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include
+
+#include "../../test/TestBase.h"
+
+#ifdef WIN32
+#include
+#define GetCurrentDir _getcwd
+#else
+#include
+#define GetCurrentDir getcwd
+#endif
+
+namespace org {
+namespace apache {
+namespace nifi {
+namespace minifi {
+namespace utils {
+
+std::string getCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+std::string createTempDir(TestController* testController) {
Review comment:
I wanted a wrapper that does not require a directory template, but
asserts on the new directory being generated.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r433931716
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -35,7 +35,11 @@ namespace python {
namespace processors {
core::Property ExecutePythonProcessor::ScriptFile("Script File", // NOLINT
-R"(Path to script file to execute)", "");
+R"(Path to script file to execute.
+Only one of Script File or Script
Body may be used)", "");
+core::Property ExecutePythonProcessor::ScriptBody("Script Body", // NOLINT
+R"(Script to execute.
+Only one of Script File or Script
Body may be used)", "");
Review comment:
I do not know, they were split by the original design. I was not sure if
this is a hack used for displaying the definition in the EFM, so I left the
split in place.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r430369227
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -46,144 +50,177 @@ core::Relationship
ExecutePythonProcessor::Failure("failure", "Script failures")
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if ("" != script_to_exec_) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
if (!valid_init_) {
-throw std::runtime_error("Could not correctly in initialize " + getName());
- }
- context->getProperty(ScriptFile.getName(), script_file_);
- context->getProperty(ModuleDirectory.getName(), module_directory_);
- if (script_file_.empty() && script_engine_.empty()) {
-logger_->log_error("Script File must be defined");
-return;
+throw std::runtime_error("Could not correctly initialize " + getName());
}
-
try {
-std::shared_ptr engine;
-
-// Use an existing engine, if one is available
-if (script_engine_q_.try_dequeue(engine)) {
- logger_->log_debug("Using available %s script engine instance",
script_engine_);
-} else {
- logger_->log_info("Creating new %s script instance", script_engine_);
- logger_->log_info("Approximately %d %s script instances created for this
processor", script_engine_q_.size_approx(), script_engine_);
-
- engine = createEngine();
-
- if (engine == nullptr) {
-throw std::runtime_error("No script engine available");
- }
-
- if (!script_file_.empty()) {
-engine->evalFile(script_file_);
- } else {
-throw std::runtime_error("No Script File is available to execute");
- }
+// TODO(hunyadi): When using "Script File" property, we currently re-read
the script file content every time the processor is on schedule. This should
change to single-read when we release 1.0.0
+// https://issues.apache.org/jira/browse/MINIFICPP-1223
+reloadScriptIfUsingScriptFileProperty();
Review comment:
1) I am happy to do the changes for this. I will raise this as a
question the next time we have a team meeting.
2) Good idea, but this change should have its own Jira.
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.
For queries about this service, please
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r431867122
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,276 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+
+namespace {
+
+#include
+#define GetCurrentDir getcwd
+
+std::string GetCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+class ExecutePythonProcessorTestBase {
+ public:
+ExecutePythonProcessorTestBase() :
+ logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+ reInitialize();
+}
+virtual ~ExecutePythonProcessorTestBase() {
+ logTestController_.reset();
+ logTestController_.setDebug();
+
logTestController_.setDebug();
+ logTestController_.setDebug();
+ logTestController_.setDebug();
+}
+
+ protected:
+void reInitialize() {
+ testController_.reset(new TestController());
+ plan_ = testController_->createPlan();
+}
+
+std::string createTempDir() {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController_->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator()
+ file_name);
+ std::ofstream out_file(file_path);
+ if (out_file.is_open()) {
+out_file << content;
+out_file.close();
+ }
+ return file_path;
+}
+
+std::string createTempDirWithFile(const std::string& file_name, const
std::string& content) {
+ std::string temp_dir = createTempDir();
+ putFileToDir(temp_dir, file_name, content);
+ return temp_dir;
+}
+
+std::string getFileContent(const std::string& file_name) {
+ std::ifstream file_handle(file_name);
+ REQUIRE(file_handle.is_open());
+ const std::string file_content{
(std::istreambuf_iterator(file_handle)),
(std::istreambuf_iterator())};
+ file_handle.close();
+ return file_content;
+}
+
+std::string getScriptFullPath(const std::string& script_file_name) {
+ return SCRIPT_FILES_DIRECTORY + utils::file::FileUtils::get_separator()
+ script_file_name;
+}
+
+const std::string TEST_FILE_NAME{ "test_file.txt" };
+const std::string TEST_FILE_CONTENT{ "Test text\n" };
+const std::string SCRIPT_FILES_DIRECTORY{ "test_scripts" };
+
+std::unique_ptr testController_;
+std::shared_ptr plan_;
+LogTestController& logTestController_;
+std::shared_ptr logger_;
+};
+
+class SimplePythonFlowFileTransferTest : public ExecutePythonProcessorTestBase
{
+ public:
+ enum class Expectation {
+OUTPUT_FILE_MATCHES_INPUT,
+RUNTIME_RELATIONSHIP_EXCEPTION,
+PROCESSOR_INITIALIZATION_EXCEPTION
+ };
+ SimplePythonFlowFileTransferTest() : ExecutePythonProcessorTestBase{} {}
+
+ protected:
+ void testSimpleFilePassthrough(const Expectation expectation, const
core::Relationship& execute_python_out_conn, const std::string&
used_as_script_file, const std::string& used_as_script_body) {
+reInitialize();
+const std::string input_dir = createTempDirWithFile(TEST_FILE_NAME,
TEST_FILE_CONTENT);
+const std::string output_dir = createTempDir();
+
+addGetFileProcessorToPlan(input_dir);
+if (Expectation::PROCESSOR_INITIALIZATION_EXCEPTION == expectation) {
+ REQUIRE_THROWS(addExecutePythonProcessorToPlan(used_as_script_file,
used_as_script_body));
+ return;
+}
+
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784: URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r431866934 ## File path: libminifi/test/script-tests/PythonExecuteScriptTests.cpp ## @@ -29,6 +29,15 @@ #include "processors/GetFile.h" #include "processors/PutFile.h" +// ,-, +// | ! | Disclaimer | ! | +// |---' '---' +// | | +// | This file contains tests for the "ExecuteScript" processor, | +// | not for the "ExecutePython" processor. | +// | | +// '-' Review comment: Renamed the tests. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r431865524
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,276 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+
+namespace {
+
+#include
+#define GetCurrentDir getcwd
Review comment:
Yes, I already said I have realized that this was there as a bug as well
:)
https://user-images.githubusercontent.com/64011968/83152173-caee3400-a0fd-11ea-8be1-bd919e286c87.png;>
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r431862488
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,276 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+
+namespace {
+
+#include
+#define GetCurrentDir getcwd
+
+std::string GetCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+class ExecutePythonProcessorTestBase {
+ public:
+ExecutePythonProcessorTestBase() :
+ logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+ reInitialize();
+}
+virtual ~ExecutePythonProcessorTestBase() {
+ logTestController_.reset();
+ logTestController_.setDebug();
+
logTestController_.setDebug();
+ logTestController_.setDebug();
+ logTestController_.setDebug();
+}
+
+ protected:
+void reInitialize() {
+ testController_.reset(new TestController());
+ plan_ = testController_->createPlan();
+}
+
+std::string createTempDir() {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController_->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator()
+ file_name);
+ std::ofstream out_file(file_path);
+ if (out_file.is_open()) {
+out_file << content;
+out_file.close();
+ }
+ return file_path;
+}
+
+std::string createTempDirWithFile(const std::string& file_name, const
std::string& content) {
+ std::string temp_dir = createTempDir();
+ putFileToDir(temp_dir, file_name, content);
+ return temp_dir;
+}
+
+std::string getFileContent(const std::string& file_name) {
+ std::ifstream file_handle(file_name);
+ REQUIRE(file_handle.is_open());
+ const std::string file_content{
(std::istreambuf_iterator(file_handle)),
(std::istreambuf_iterator())};
+ file_handle.close();
+ return file_content;
+}
Review comment:
I would argue that the workflow would not be any more reusable, as a
having a `TestController` is a dependency either way. Also, I would rather have
the REQUIRE assertions inside the test-helper function for two reasons:
- Someone can easily forget adding EXPECT_THROW on the call side ending up
accidentaly satisfying "EXPECT_THROW" calls on an upper call stack.
- Adding the expectation for throwing would make the code would look like
we test the helper functions inside of unit tests, and might not be obvious why
is there an expectation for not throwing for a given test.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r431862488
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,276 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+
+namespace {
+
+#include
+#define GetCurrentDir getcwd
+
+std::string GetCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+class ExecutePythonProcessorTestBase {
+ public:
+ExecutePythonProcessorTestBase() :
+ logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+ reInitialize();
+}
+virtual ~ExecutePythonProcessorTestBase() {
+ logTestController_.reset();
+ logTestController_.setDebug();
+
logTestController_.setDebug();
+ logTestController_.setDebug();
+ logTestController_.setDebug();
+}
+
+ protected:
+void reInitialize() {
+ testController_.reset(new TestController());
+ plan_ = testController_->createPlan();
+}
+
+std::string createTempDir() {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController_->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator()
+ file_name);
+ std::ofstream out_file(file_path);
+ if (out_file.is_open()) {
+out_file << content;
+out_file.close();
+ }
+ return file_path;
+}
+
+std::string createTempDirWithFile(const std::string& file_name, const
std::string& content) {
+ std::string temp_dir = createTempDir();
+ putFileToDir(temp_dir, file_name, content);
+ return temp_dir;
+}
+
+std::string getFileContent(const std::string& file_name) {
+ std::ifstream file_handle(file_name);
+ REQUIRE(file_handle.is_open());
+ const std::string file_content{
(std::istreambuf_iterator(file_handle)),
(std::istreambuf_iterator())};
+ file_handle.close();
+ return file_content;
+}
Review comment:
1. I would argue that the workflow would not be any more reusable, as a
having a `TestController` is a dependency either way. Also, I would rather have
the REQUIRE assertions inside the test-helper function for two reasons:
- Someone can easily forget adding EXPECT_THROW on the call side ending up
accidentaly satisfying "EXPECT_THROW" calls on an upper call stack.
- Adding the expectation for throwing would make the code would look like
we test the helper functions inside of unit tests, and might not be obvious why
is there an expectation for not throwing for a given test.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r431855542
##
File path: libminifi/test/script-tests/ExecutePythonProcessorTests.cpp
##
@@ -0,0 +1,276 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#define CATCH_CONFIG_MAIN
+
+#include
+#include
+#include
+
+#include "../TestBase.h"
+
+#include "processors/GetFile.h"
+#include "python/ExecutePythonProcessor.h"
+#include "processors/LogAttribute.h"
+#include "processors/PutFile.h"
+#include "utils/file/FileUtils.h"
+
+namespace {
+
+#include
+#define GetCurrentDir getcwd
+
+std::string GetCurrentWorkingDir(void) {
+ char buff[FILENAME_MAX];
+ GetCurrentDir(buff, FILENAME_MAX);
+ std::string current_working_dir(buff);
+ return current_working_dir;
+}
+
+class ExecutePythonProcessorTestBase {
+ public:
+ExecutePythonProcessorTestBase() :
+ logTestController_(LogTestController::getInstance()),
+
logger_(logging::LoggerFactory::getLogger())
{
+ reInitialize();
+}
+virtual ~ExecutePythonProcessorTestBase() {
+ logTestController_.reset();
+ logTestController_.setDebug();
+
logTestController_.setDebug();
+ logTestController_.setDebug();
+ logTestController_.setDebug();
+}
+
+ protected:
+void reInitialize() {
+ testController_.reset(new TestController());
+ plan_ = testController_->createPlan();
+}
+
+std::string createTempDir() {
+ char dirtemplate[] = "/tmp/gt.XX";
+ std::string temp_dir = testController_->createTempDirectory(dirtemplate);
+ REQUIRE(!temp_dir.empty());
+ struct stat buffer;
+ REQUIRE(-1 != stat(temp_dir.c_str(), ));
+ REQUIRE(S_ISDIR(buffer.st_mode));
+ return temp_dir;
+}
+
+std::string putFileToDir(const std::string& dir_path, const std::string&
file_name, const std::string& content) {
+ std::string file_path(dir_path + utils::file::FileUtils::get_separator()
+ file_name);
+ std::ofstream out_file(file_path);
+ if (out_file.is_open()) {
+out_file << content;
+out_file.close();
+ }
+ return file_path;
+}
+
+std::string createTempDirWithFile(const std::string& file_name, const
std::string& content) {
+ std::string temp_dir = createTempDir();
+ putFileToDir(temp_dir, file_name, content);
+ return temp_dir;
+}
+
+std::string getFileContent(const std::string& file_name) {
+ std::ifstream file_handle(file_name);
+ REQUIRE(file_handle.is_open());
+ const std::string file_content{
(std::istreambuf_iterator(file_handle)),
(std::istreambuf_iterator())};
+ file_handle.close();
+ return file_content;
+}
+
+std::string getScriptFullPath(const std::string& script_file_name) {
+ return SCRIPT_FILES_DIRECTORY + utils::file::FileUtils::get_separator()
+ script_file_name;
+}
+
+const std::string TEST_FILE_NAME{ "test_file.txt" };
+const std::string TEST_FILE_CONTENT{ "Test text\n" };
+const std::string SCRIPT_FILES_DIRECTORY{ "test_scripts" };
Review comment:
I mean the initialization of this static member goes outside the class,
so if people check the declaration they won't immediately see these values.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r430983396
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -46,144 +50,177 @@ core::Relationship
ExecutePythonProcessor::Failure("failure", "Script failures")
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if ("" != script_to_exec_) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
if (!valid_init_) {
-throw std::runtime_error("Could not correctly in initialize " + getName());
- }
- context->getProperty(ScriptFile.getName(), script_file_);
- context->getProperty(ModuleDirectory.getName(), module_directory_);
- if (script_file_.empty() && script_engine_.empty()) {
-logger_->log_error("Script File must be defined");
-return;
+throw std::runtime_error("Could not correctly initialize " + getName());
}
-
try {
-std::shared_ptr engine;
-
-// Use an existing engine, if one is available
-if (script_engine_q_.try_dequeue(engine)) {
- logger_->log_debug("Using available %s script engine instance",
script_engine_);
-} else {
- logger_->log_info("Creating new %s script instance", script_engine_);
- logger_->log_info("Approximately %d %s script instances created for this
processor", script_engine_q_.size_approx(), script_engine_);
-
- engine = createEngine();
-
- if (engine == nullptr) {
-throw std::runtime_error("No script engine available");
- }
-
- if (!script_file_.empty()) {
-engine->evalFile(script_file_);
- } else {
-throw std::runtime_error("No Script File is available to execute");
- }
+// TODO(hunyadi): When using "Script File" property, we currently re-read
the script file content every time the processor is on schedule. This should
change to single-read when we release 1.0.0
+// https://issues.apache.org/jira/browse/MINIFICPP-1223
+reloadScriptIfUsingScriptFileProperty();
Review comment:
Jira added here: https://issues.apache.org/jira/browse/MINIFICPP-1240
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] hunyadi-dev commented on a change in pull request #784: MINIFICPP-1206 - Rework and test ExecutePythonProcessor, add in-place script support
hunyadi-dev commented on a change in pull request #784:
URL: https://github.com/apache/nifi-minifi-cpp/pull/784#discussion_r430365464
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -46,144 +50,177 @@ core::Relationship
ExecutePythonProcessor::Failure("failure", "Script failures")
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if ("" != script_to_exec_) {
Review comment:
I think this is more expressive, as it shows we are handling an
`std::string` as opposed to `std::vector`. Also even on reading
this comment I missed the bang on the first option, and `not` is warned on
(unreliably though) by the linter.
##
File path: extensions/script/python/ExecutePythonProcessor.cpp
##
@@ -46,144 +50,177 @@ core::Relationship
ExecutePythonProcessor::Failure("failure", "Script failures")
void ExecutePythonProcessor::initialize() {
// initialization requires that we do a little leg work prior to onSchedule
// so that we can provide manifest our processor identity
- std::set properties;
-
- std::string prop;
- getProperty(ScriptFile.getName(), prop);
-
- properties.insert(ScriptFile);
- properties.insert(ModuleDirectory);
- setSupportedProperties(properties);
-
- std::set relationships;
- relationships.insert(Success);
- relationships.insert(Failure);
- setSupportedRelationships(std::move(relationships));
- setAcceptAllProperties();
- if (!prop.empty()) {
-setProperty(ScriptFile, prop);
-std::shared_ptr engine;
-python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
+ if (getProperties().empty()) {
+setSupportedProperties({
+ ScriptFile,
+ ScriptBody,
+ ModuleDirectory
+});
+setAcceptAllProperties();
+setSupportedRelationships({
+ Success,
+ Failure
+});
+valid_init_ = false;
+return;
+ }
-engine = createEngine();
+ python_logger_ =
logging::LoggerFactory::getAliasedLogger(getName());
-if (engine == nullptr) {
- throw std::runtime_error("No script engine available");
-}
+ getProperty(ModuleDirectory.getName(), module_directory_);
-try {
- engine->evalFile(prop);
- auto me = shared_from_this();
- triggerDescribe(engine, me);
- triggerInitialize(engine, me);
+ valid_init_ = false;
+ appendPathForImportModules();
+ loadScript();
+ try {
+if ("" != script_to_exec_) {
+ std::shared_ptr engine = getScriptEngine();
+ engine->eval(script_to_exec_);
+ auto shared_this = shared_from_this();
+ engine->describe(shared_this);
+ engine->onInitialize(shared_this);
+ handleEngineNoLongerInUse(std::move(engine));
valid_init_ = true;
-} catch (std::exception ) {
- logger_->log_error("Caught Exception %s", exception.what());
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
-} catch (...) {
- logger_->log_error("Caught Exception");
- engine = nullptr;
- std::rethrow_exception(std::current_exception());
- valid_init_ = false;
}
-
+ }
+ catch (const std::exception& exception) {
+logger_->log_error("Caught Exception: %s", exception.what());
+std::rethrow_exception(std::current_exception());
+ }
+ catch (...) {
+logger_->log_error("Caught Exception");
+std::rethrow_exception(std::current_exception());
}
}
void ExecutePythonProcessor::onSchedule(const
std::shared_ptr , const
std::shared_ptr ) {
