szaszm commented on code in PR #1712:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1712#discussion_r1459074581
##########
libminifi/include/core/ProcessContext.h:
##########
@@ -135,6 +135,10 @@ class ProcessContext : public
controller::ControllerServiceLookup, public core::
return getProperty(property.name, value);
}
+ virtual bool getProperty(bool /*supports_expression_language*/,
std::string_view property_name, std::string& value, const
std::shared_ptr<FlowFile>& /*flow_file*/) {
+ return getProperty(property_name, value);
+ }
Review Comment:
Why do we need a new virtual overload? I'd prefer a single virtual function
as an customization point, and possibly other non-static overloads to handle
different use cases, to keep things simple.
##########
docker/test/integration/features/environment.py:
##########
@@ -47,6 +47,13 @@ def before_scenario(context, scenario):
logging.info("Integration test setup at
{time:%H:%M:%S.%f}".format(time=datetime.datetime.now()))
context.test = MiNiFi_integration_test(context=context,
feature_id=context.feature_id)
+
+ if "USE_NIFI_PYTHON_PROCESSORS" in scenario.effective_tags:
+ if not context.image_store.is_conda_available_in_minifi_image() and
context.image_store.get_minifi_image_python_version() < (3, 8, 1):
+ scenario.skip("NiFi Python processor tests use langchain library
therefore Python 3.8.1 or later is needed in the MiNiFi container.")
Review Comment:
I'd rephrase this.
```suggestion
scenario.skip("NiFi Python processor tests use langchain library
which requires Python 3.8.1 or later.")
```
##########
libminifi/include/core/ConfigurableComponent.h:
##########
@@ -212,6 +212,8 @@ class ConfigurableComponent {
// Dynamic properties
std::map<std::string, Property> dynamic_properties_;
+ virtual Property* findProperty(const std::string& name) const;
+
Review Comment:
Doesn't getProperty already handle this use case?
##########
extensions/python/PythonProcessor.h:
##########
@@ -37,7 +38,7 @@ class PythonProcessor {
void setDescription(const std::string& desc);
- void addProperty(const std::string& name, const std::string& description,
const std::string& defaultvalue, bool required, bool el);
+ void addProperty(const std::string& name, const std::string& description,
const std::optional<std::string>& defaultvalue, bool required, bool el, const
std::optional<int64_t>& validator_value);
Review Comment:
What's a validator value?
##########
extensions/python/tests/PythonManifestTests.cpp:
##########
@@ -101,7 +101,7 @@ TEST_CASE("Python processor's description is part of the
manifest") {
REQUIRE(getNode(MyPyProc->children, "type").value ==
"org.apache.nifi.minifi.processors.MyPyProc");
auto& rels = getNode(MyPyProc->children,
"supportedRelationships").children;
- REQUIRE(rels.size() == 2);
+ REQUIRE(rels.size() == 3);
Review Comment:
What changed here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
