This is an automated email from the ASF dual-hosted git repository. lordgamez pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/nifi-minifi-cpp.git
commit 3e9911409963a53c43494d5ff0bbafbfe8d40a47 Author: Ferenc Gerlits <[email protected]> AuthorDate: Thu Aug 15 14:39:28 2024 +0200 MINIFICPP-2437 Include allowable_values in the manifest for python processors Signed-off-by: Gabor Gyimesi <[email protected]> This closes #1858 --- extensions/python/ExecutePythonProcessor.cpp | 19 ++++++----- extensions/python/ExecutePythonProcessor.h | 7 ++-- extensions/python/PYTHON.md | 1 - extensions/python/PythonProcessor.cpp | 9 +++-- extensions/python/PythonProcessor.h | 6 ++-- .../pythonprocessors/nifiapi/processorbase.py | 2 +- extensions/python/tests/PythonManifestTests.cpp | 13 +++++++- extensions/python/types/PyProcessor.cpp | 39 ++++++++++++++-------- extensions/python/types/Types.h | 8 ++++- libminifi/include/core/Property.h | 12 ++----- libminifi/src/core/Property.cpp | 4 +++ 11 files changed, 74 insertions(+), 46 deletions(-) diff --git a/extensions/python/ExecutePythonProcessor.cpp b/extensions/python/ExecutePythonProcessor.cpp index a1541498a..7beca34d4 100644 --- a/extensions/python/ExecutePythonProcessor.cpp +++ b/extensions/python/ExecutePythonProcessor.cpp @@ -165,22 +165,25 @@ std::unique_ptr<PythonScriptEngine> ExecutePythonProcessor::createScriptEngine() return engine; } -void ExecutePythonProcessor::addProperty(const std::string &name, const std::string &description, const std::optional<std::string> &defaultvalue, bool required, bool el, - bool sensitive, const std::optional<int64_t>& property_type_code, const std::optional<std::string>& controller_service_type_name) { - auto property = core::PropertyDefinitionBuilder<>::createProperty(name).withDescription(description).isRequired(required).supportsExpressionLanguage(el).isSensitive(sensitive); +void ExecutePythonProcessor::addProperty(const std::string &name, const std::string &description, const std::optional<std::string> &defaultvalue, bool required, bool el, bool sensitive, + const std::optional<int64_t>& property_type_code, gsl::span<const std::string_view> allowable_values, const std::optional<std::string>& controller_service_type_name) { + auto builder = core::PropertyDefinitionBuilder<>::createProperty(name).withDescription(description).isRequired(required).supportsExpressionLanguage(el).isSensitive(sensitive); if (defaultvalue) { - property.withDefaultValue(*defaultvalue); + builder.withDefaultValue(*defaultvalue); } if (property_type_code) { - property.withPropertyType(core::StandardPropertyTypes::translateCodeToPropertyType(static_cast<core::StandardPropertyTypes::PropertyTypeCode>(*property_type_code))); + builder.withPropertyType(core::StandardPropertyTypes::translateCodeToPropertyType(static_cast<core::StandardPropertyTypes::PropertyTypeCode>(*property_type_code))); } - if (controller_service_type_name && *controller_service_type_name == "SSLContextService") { - property.withAllowedTypes<controllers::SSLContextService>(); + builder.withAllowedTypes<controllers::SSLContextService>(); } + const auto property_definition = builder.build(); + + core::Property property{property_definition}; + property.setAllowedValues(allowable_values, *property_definition.type); std::lock_guard<std::mutex> lock(python_properties_mutex_); - python_properties_.emplace_back(property.build()); + python_properties_.emplace_back(property); } const core::Property* ExecutePythonProcessor::findProperty(const std::string& name) const { diff --git a/extensions/python/ExecutePythonProcessor.h b/extensions/python/ExecutePythonProcessor.h index b87a5ee47..f344f35af 100644 --- a/extensions/python/ExecutePythonProcessor.h +++ b/extensions/python/ExecutePythonProcessor.h @@ -22,6 +22,7 @@ #include <memory> #include <string> +#include <string_view> #include <utility> #include <vector> #include <filesystem> @@ -33,7 +34,7 @@ #include "core/PropertyType.h" #include "core/RelationshipDefinition.h" #include "PythonScriptEngine.h" -#include "PythonScriptEngine.h" +#include "utils/gsl.h" namespace org::apache::nifi::minifi::extensions::python::processors { @@ -95,8 +96,8 @@ class ExecutePythonProcessor : public core::Processor { python_dynamic_ = true; } - void addProperty(const std::string &name, const std::string &description, const std::optional<std::string> &defaultvalue, bool required, bool el, - bool sensitive, const std::optional<int64_t>& property_type_code, const std::optional<std::string>& controller_service_type_name); + void addProperty(const std::string &name, const std::string &description, const std::optional<std::string> &defaultvalue, bool required, bool el, bool sensitive, + const std::optional<int64_t>& property_type_code, gsl::span<const std::string_view> allowable_values, const std::optional<std::string>& controller_service_type_name); std::vector<core::Property> getPythonProperties() const { std::lock_guard<std::mutex> lock(python_properties_mutex_); diff --git a/extensions/python/PYTHON.md b/extensions/python/PYTHON.md index e0482709f..2b0f7c076 100644 --- a/extensions/python/PYTHON.md +++ b/extensions/python/PYTHON.md @@ -164,7 +164,6 @@ In the flow configuration these Python processors can be referenced by their ful Due to some differences between the NiFi and MiNiFi C++ processors and implementation, there are some limitations using the NiFi Python processors: - Record based processors are not yet supported in MiNiFi C++, so the NiFi Python processors inherited from RecordTransform are not supported. - There are some validators in NiFi that are not present in MiNiFi C++, so some property validations will be missing using the NiFi Python processors. -- Allowable values specified in NiFi Python processors are ignored in MiNiFi C++ (due to MiNiFi C++ requiring them to be specified at compile time), so the property values are not pre-verified. - MiNiFi C++ only supports expression language with flow file attributes, so only FLOWFILE_ATTRIBUTES expression language scope is supported, otherwise the expression language will not be evaluated. - MiNiFi C++ does not support property dependencies, so the property dependencies will be ignored. If a property depends on another property, the property will not be required. - MiNiFi C++ does not support the use of self.jvm member in Python processors that provides JVM bindings in NiFi, it is set to None in MiNiFi C++. diff --git a/extensions/python/PythonProcessor.cpp b/extensions/python/PythonProcessor.cpp index f7097a5b4..021c1e268 100644 --- a/extensions/python/PythonProcessor.cpp +++ b/extensions/python/PythonProcessor.cpp @@ -16,10 +16,9 @@ * limitations under the License. */ -#include <string> +#include "PythonProcessor.h" #include "ExecutePythonProcessor.h" -#include "PythonProcessor.h" namespace org::apache::nifi::minifi::extensions::python { @@ -38,9 +37,9 @@ void PythonProcessor::setDescription(const std::string& desc) { processor_->setDescription(desc); } -void PythonProcessor::addProperty(const std::string& name, const std::string& description, const std::optional<std::string>& defaultvalue, - bool required, bool el, bool sensitive, const std::optional<int64_t>& property_type_code, const std::optional<std::string>& controller_service_type_name) { - processor_->addProperty(name, description, defaultvalue, required, el, sensitive, property_type_code, controller_service_type_name); +void PythonProcessor::addProperty(const std::string& name, const std::string& description, const std::optional<std::string>& defaultvalue, bool required, bool el, bool sensitive, + const std::optional<int64_t>& property_type_code, gsl::span<const std::string_view> allowable_values, const std::optional<std::string>& controller_service_type_name) { + processor_->addProperty(name, description, defaultvalue, required, el, sensitive, property_type_code, allowable_values, controller_service_type_name); } } // namespace org::apache::nifi::minifi::extensions::python diff --git a/extensions/python/PythonProcessor.h b/extensions/python/PythonProcessor.h index 135effbd2..52a7b2a8b 100644 --- a/extensions/python/PythonProcessor.h +++ b/extensions/python/PythonProcessor.h @@ -19,10 +19,12 @@ #pragma once #include <string> +#include <string_view> #include <memory> #include <optional> #include "core/Processor.h" +#include "utils/gsl.h" namespace org::apache::nifi::minifi::extensions::python { @@ -38,8 +40,8 @@ class PythonProcessor { void setDescription(const std::string& desc); - void addProperty(const std::string& name, const std::string& description, const std::optional<std::string>& defaultvalue, - bool required, bool el, bool sensitive, const std::optional<int64_t>& property_type_code, const std::optional<std::string>& controller_service_type_name); + void addProperty(const std::string& name, const std::string& description, const std::optional<std::string>& defaultvalue, bool required, bool el, bool sensitive, + const std::optional<int64_t>& property_type_code, gsl::span<const std::string_view> allowable_values, const std::optional<std::string>& controller_service_type_name); private: python::processors::ExecutePythonProcessor* processor_; diff --git a/extensions/python/pythonprocessors/nifiapi/processorbase.py b/extensions/python/pythonprocessors/nifiapi/processorbase.py index a343a4cb9..ee1034e7c 100644 --- a/extensions/python/pythonprocessors/nifiapi/processorbase.py +++ b/extensions/python/pythonprocessors/nifiapi/processorbase.py @@ -51,7 +51,7 @@ class ProcessorBase(ABC): # MiNiFi C++ does not support dependant properties, so if a property depends on another property, it should not be required is_required = True if property.required and not property.dependencies else False processor.addProperty(property.name, property.description, property.defaultValue, is_required, expression_language_supported, - property.sensitive, property_type_code, property.controllerServiceDefinition) + property.sensitive, property_type_code, property.allowableValues, property.controllerServiceDefinition) def onScheduled(self, context_proxy: ProcessContextProxy): pass diff --git a/extensions/python/tests/PythonManifestTests.cpp b/extensions/python/tests/PythonManifestTests.cpp index 61b76e43b..b0e58619a 100644 --- a/extensions/python/tests/PythonManifestTests.cpp +++ b/extensions/python/tests/PythonManifestTests.cpp @@ -27,6 +27,7 @@ #include "utils/gsl.h" using minifi::state::response::SerializedResponseNode; +using minifi::state::response::ValueNode; template<typename F> const SerializedResponseNode* findNode(const std::vector<SerializedResponseNode>& nodes, F&& filter) { @@ -43,6 +44,10 @@ const SerializedResponseNode& getNode(const std::vector<SerializedResponseNode>& gsl_FailFast(); } +ValueNode getNthAllowableValue(const SerializedResponseNode& node, size_t n) { + return getNode(node.children[n].children, "value").value; +} + TEST_CASE("Python processor's description is part of the manifest") { TestControllerWithFlow controller(empty_flow, false /* DEFER FLOW SETUP */); @@ -56,7 +61,7 @@ TEST_CASE("Python processor's description is part of the manifest") { "def describe(proc):\n" " proc.setDescription('Another amazing processor')\n" " proc.setSupportsDynamicProperties()\n" - " proc.addProperty('Prop1', 'A great property', 'banana', True, False)\n"; + " proc.addProperty('Prop1', 'A great property', 'banana', True, False, False, None, ['apple', 'orange', 'banana', 'durian'], None)\n"; controller.configuration_->set(minifi::Configuration::nifi_python_processor_dir, python_dir.string()); controller.configuration_->set(minifi::Configuration::nifi_extension_path, "*minifi-python-script*"); @@ -130,6 +135,12 @@ TEST_CASE("Python processor's description is part of the manifest") { REQUIRE(getNode(properties[0].children, "required").value == true); REQUIRE(getNode(properties[0].children, "expressionLanguageScope").value == "NONE"); REQUIRE(getNode(properties[0].children, "defaultValue").value == "banana"); + auto& allowable_values = getNode(properties[0].children, "allowableValues"); + REQUIRE(allowable_values.children.size() == 4); + CHECK(getNthAllowableValue(allowable_values, 0) == "apple"); + CHECK(getNthAllowableValue(allowable_values, 1) == "orange"); + CHECK(getNthAllowableValue(allowable_values, 2) == "banana"); + CHECK(getNthAllowableValue(allowable_values, 3) == "durian"); auto& rels = getNode(MyPyProc2->children, "supportedRelationships").children; REQUIRE(rels.size() == 3); diff --git a/extensions/python/types/PyProcessor.cpp b/extensions/python/types/PyProcessor.cpp index e951731d4..dd8ad17b2 100644 --- a/extensions/python/types/PyProcessor.cpp +++ b/extensions/python/types/PyProcessor.cpp @@ -110,6 +110,13 @@ PyObject* PyProcessor::addProperty(PyProcessor* self, PyObject* args) { return nullptr; } + static constexpr Py_ssize_t ExpectedNumArgs = 9; + auto arg_size = PyTuple_Size(args); + if (arg_size < ExpectedNumArgs) { + PyErr_SetString(PyExc_AttributeError, fmt::format("addProperty was called with too few arguments: need {}, got {}", ExpectedNumArgs, arg_size).c_str()); + return nullptr; + } + BorrowedStr name = BorrowedStr::fromTuple(args, 0); BorrowedStr description = BorrowedStr::fromTuple(args, 1); std::optional<std::string> default_value; @@ -121,36 +128,40 @@ PyObject* PyProcessor::addProperty(PyProcessor* self, PyObject* args) { bool is_required = false; bool supports_expression_language = false; bool sensitive = false; - auto arg_size = PyTuple_Size(args); try { is_required = getBoolFromTuple(args, 3); supports_expression_language = getBoolFromTuple(args, 4); - if (arg_size > 5) { - sensitive = getBoolFromTuple(args, 5); - } + sensitive = getBoolFromTuple(args, 5); } catch (const PyException&) { return nullptr; } std::optional<int64_t> validator_value; - if (arg_size > 6) { - auto validator_value_pyint = BorrowedLong::fromTuple(args, 6); - if (validator_value_pyint.get() && validator_value_pyint.get() != Py_None) { - validator_value = validator_value_pyint.asInt64(); + auto validator_value_pyint = BorrowedLong::fromTuple(args, 6); + if (validator_value_pyint.get() && validator_value_pyint.get() != Py_None) { + validator_value = validator_value_pyint.asInt64(); + } + + std::vector<std::string> allowable_values_str; + auto allowable_values_pylist = BorrowedList::fromTuple(args, 7); + if (allowable_values_pylist.get() && allowable_values_pylist.get() != Py_None) { + for (size_t i = 0; i < allowable_values_pylist.length(); ++i) { + auto value = BorrowedStr{allowable_values_pylist[i]}; + allowable_values_str.push_back(value.toUtf8String()); } } + std::vector<std::string_view> allowable_values(begin(allowable_values_str), end(allowable_values_str)); std::optional<std::string> controller_service_type_name; - if (arg_size > 7) { - auto controller_service_type_name_pystr = BorrowedStr::fromTuple(args, 7); - if (controller_service_type_name_pystr.get() && controller_service_type_name_pystr.get() != Py_None) { - controller_service_type_name = controller_service_type_name_pystr.toUtf8String(); - } + auto controller_service_type_name_pystr = BorrowedStr::fromTuple(args, 8); + if (controller_service_type_name_pystr.get() && controller_service_type_name_pystr.get() != Py_None) { + controller_service_type_name = controller_service_type_name_pystr.toUtf8String(); } - processor->addProperty(name.toUtf8String(), description.toUtf8String(), default_value, is_required, supports_expression_language, sensitive, validator_value, controller_service_type_name); + processor->addProperty(name.toUtf8String(), description.toUtf8String(), default_value, is_required, supports_expression_language, sensitive, + validator_value, allowable_values, controller_service_type_name); Py_RETURN_NONE; } diff --git a/extensions/python/types/Types.h b/extensions/python/types/Types.h index 2aebadd1b..0c14039f2 100644 --- a/extensions/python/types/Types.h +++ b/extensions/python/types/Types.h @@ -254,8 +254,14 @@ class List : public ReferenceHolder<reference_type> { } return BorrowedReference(item); } -}; + static BorrowedList fromTuple(PyObject* tuple, Py_ssize_t location) requires(reference_type == ReferenceType::BORROWED) { + BorrowedList list_from_tuple{PyTuple_GetItem(tuple, location)}; + if (list_from_tuple.get() == nullptr) + throw PyException(); + return list_from_tuple; + } +}; template<ReferenceType reference_type> class Dict : public ReferenceHolder<reference_type> { diff --git a/libminifi/include/core/Property.h b/libminifi/include/core/Property.h index 56c01587f..ba3001655 100644 --- a/libminifi/include/core/Property.h +++ b/libminifi/include/core/Property.h @@ -145,18 +145,10 @@ class Property { return allowed_values_; } - void addAllowedValue(const PropertyValue &value) { - allowed_values_.push_back(value); - } - - void clearAllowedValues() { - allowed_values_.clear(); - } + void setAllowedValues(gsl::span<const std::string_view> allowed_values, const core::PropertyParser& property_parser); - /** - * Add value to the collection of values. - */ void addValue(const std::string &value); + Property &operator=(const Property &other) = default; Property &operator=(Property &&other) = default; // Compare diff --git a/libminifi/src/core/Property.cpp b/libminifi/src/core/Property.cpp index 1b49469d1..4b5186a40 100644 --- a/libminifi/src/core/Property.cpp +++ b/libminifi/src/core/Property.cpp @@ -131,4 +131,8 @@ Property::Property(const PropertyReference& compile_time_property) } } +void Property::setAllowedValues(gsl::span<const std::string_view> allowed_values, const core::PropertyParser& property_parser) { + allowed_values_ = createPropertyValues(allowed_values, property_parser); +} + } // namespace org::apache::nifi::minifi::core
