szaszm commented on code in PR #1712:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1712#discussion_r1505967478


##########
extensions/python/ExecutePythonProcessor.cpp:
##########
@@ -44,8 +45,7 @@ void ExecutePythonProcessor::initialize() {
 
   try {
     loadScript();
-  } catch(const std::runtime_error&) {
-    logger_->log_warn("Could not load python script while initializing. In 
case of non-native python processor this is normal and will be done in the 
schedule phase.");
+  } catch(const std::runtime_error& err) {

Review Comment:
   I'd keep this unnamed as long as it's not referenced.
   ```suggestion
     } catch(const std::runtime_error&) {
   ```



##########
extensions/python/PythonScriptEngine.cpp:
##########
@@ -180,4 +198,66 @@ void PythonScriptEngine::evaluateModuleImports() {
   }
 }
 
+void PythonScriptEngine::initializeProcessorObject(const std::string& 
python_class_name) {
+  GlobalInterpreterLock gil;
+  if (auto python_class = bindings_[python_class_name]) {
+    auto num_args = [&]() -> size_t {
+      auto class_init = 
OwnedObject(PyObject_GetAttrString(python_class->get(), "__init__"));
+      if (!class_init.get()) {
+        return 0;
+      }
+
+      auto inspect_module = OwnedObject(PyImport_ImportModule("inspect"));
+      if (!inspect_module.get()) {
+        return 0;
+      }
+
+      auto inspect_args = 
OwnedObject(PyObject_CallMethod(inspect_module.get(), "getfullargspec", "O", 
class_init.get()));
+      if (!inspect_args.get()) {
+        return 0;
+      }
+
+      auto arg_list = OwnedObject(PyObject_GetAttrString(inspect_args.get(), 
"args"));
+      if (!arg_list.get()) {
+        return 0;
+      }
+
+      return PyList_Size(arg_list.get());
+    }();
+
+    if (num_args > 1) {
+      auto kwargs = OwnedDict::create();
+      auto value = OwnedObject(Py_None);
+      kwargs.put("jvm", value);
+      auto args = OwnedObject(PyTuple_New(0));
+      processor_instance_ = OwnedObject(PyObject_Call(python_class->get(), 
args.get(), kwargs.get()));
+    } else {
+      processor_instance_ = 
OwnedObject(PyObject_CallObject(python_class->get(), nullptr));
+    }
+
+    if (processor_instance_.get() == nullptr) {
+      throw PythonScriptException(PyException().what());
+    }
+
+    auto result = PyObject_SetAttrString(processor_instance_.get(), "logger", 
bindings_[std::string("log")]->get());

Review Comment:
   Why are the keys wrapped in `std::string`? String literals are `const char*` 
and are already null-terminated, so if that's the only reason, I think they 
should be just raw literals.
   
   The interface is confusing and dangerous, taking `std::string_view` when it 
needs a null-terminated string internally, and we should change that in my 
opinion, but not in this PR. Created Jira for this, but let me know if I'm 
missing something about this: 
[MINIFICPP-2308](https://issues.apache.org/jira/browse/MINIFICPP-2308)



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

To unsubscribe, e-mail: [email protected]

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

Reply via email to