Re: [PR] MINIFICPP-2295 Add controller service support for NiFi python processors [nifi-minifi-cpp]

2024-04-30 Thread via GitHub


fgerlits commented on code in PR #1762:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1762#discussion_r1584692377


##
extensions/python/types/PyProcessContext.cpp:
##
@@ -104,6 +107,30 @@ PyObject* 
PyProcessContext::getStateManager(PyProcessContext* self, PyObject*) {
   return object::returnReference(context->getStateManager());
 }
 
+PyObject* PyProcessContext::getControllerService(PyProcessContext* self, 
PyObject* args) {
+  auto context = self->process_context_.lock();
+  if (!context) {
+PyErr_SetString(PyExc_AttributeError, "tried reading process context 
outside 'on_trigger'");
+return nullptr;
+  }
+
+  const char* controller_service_name = nullptr;
+  const char* controller_service_type = nullptr;
+  if (!PyArg_ParseTuple(args, "s|s", _service_name, 
_service_type)) {
+throw PyException();
+  }
+
+  if (auto controller_service = 
context->getControllerService(controller_service_name)) {
+std::string controller_service_type_str = controller_service_type;
+if (controller_service_type_str == "SSLContextService") {
+  auto ssl_ctx_service = 
std::dynamic_pointer_cast(controller_service);
+  return object::returnReference(std::weak_ptr(ssl_ctx_service));
+}
+  }

Review Comment:
   Since we only support `SSLContextService` for now, I think we should change 
the titles (commit, Jira, PR) to reflect this, eg. "Add SSLContextService 
support for NiFi python processors".
   
   Also, we could log an error or warning here when the requested controller 
service type is something other than `SSLContextService`.



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINIFICPP-2295 Add controller service support for NiFi python processors [nifi-minifi-cpp]

2024-04-30 Thread via GitHub


fgerlits commented on code in PR #1762:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1762#discussion_r1584692377


##
extensions/python/types/PyProcessContext.cpp:
##
@@ -104,6 +107,30 @@ PyObject* 
PyProcessContext::getStateManager(PyProcessContext* self, PyObject*) {
   return object::returnReference(context->getStateManager());
 }
 
+PyObject* PyProcessContext::getControllerService(PyProcessContext* self, 
PyObject* args) {
+  auto context = self->process_context_.lock();
+  if (!context) {
+PyErr_SetString(PyExc_AttributeError, "tried reading process context 
outside 'on_trigger'");
+return nullptr;
+  }
+
+  const char* controller_service_name = nullptr;
+  const char* controller_service_type = nullptr;
+  if (!PyArg_ParseTuple(args, "s|s", _service_name, 
_service_type)) {
+throw PyException();
+  }
+
+  if (auto controller_service = 
context->getControllerService(controller_service_name)) {
+std::string controller_service_type_str = controller_service_type;
+if (controller_service_type_str == "SSLContextService") {
+  auto ssl_ctx_service = 
std::dynamic_pointer_cast(controller_service);
+  return object::returnReference(std::weak_ptr(ssl_ctx_service));
+}
+  }

Review Comment:
   Since we only support `SSLContextService` for now, I think we should change 
the titles (commit, Jira, PR) to reflect this, eg. "Add SSLContextService 
support for NiFi python processors".



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINIFICPP-2295 Add controller service support for NiFi python processors [nifi-minifi-cpp]

2024-04-23 Thread via GitHub


lordgamez commented on code in PR #1762:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1762#discussion_r1575788821


##
extensions/python/PYTHON.md:
##
@@ -102,10 +102,12 @@ NON_BLANK = 5
 PORT = 6
 ```
 
+The last parameter of addProperty is the controller service type. If the 
property is a controller service, the controller service type should be 
provided. It should be the non-qualified type name of the controller service. 
Currently SSLContextService is the only controller service type supported.
+
 ```python
 def onInitialize(processor):
   processor.setSupportsDynamicProperties()
-  processor.addProperty("property name","description","default value", True 
/*required*/, False /*expression language supported*/, False /*sensitive*/, 1 
/*property type code*/)
+  processor.addProperty("property name", "description", "default value", True 
/*required*/, False /*expression language supported*/, False /*sensitive*/, 1 
/*property type code*/, None /*controller service type name*/)

Review Comment:
   Good point, I added comment for the arguments in 
0b35f1cf419e8b57955f65f9192cf39b8d8a5fde. Due to the implementation of 
`PyProcessor::addProperty(PyProcessor* self, PyObject* args)`  these are 
handled as positional arguments, so using keyword arguments would be misleading.



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



Re: [PR] MINIFICPP-2295 Add controller service support for NiFi python processors [nifi-minifi-cpp]

2024-04-22 Thread via GitHub


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


##
extensions/python/PYTHON.md:
##
@@ -102,10 +102,12 @@ NON_BLANK = 5
 PORT = 6
 ```
 
+The last parameter of addProperty is the controller service type. If the 
property is a controller service, the controller service type should be 
provided. It should be the non-qualified type name of the controller service. 
Currently SSLContextService is the only controller service type supported.
+
 ```python
 def onInitialize(processor):
   processor.setSupportsDynamicProperties()
-  processor.addProperty("property name","description","default value", True 
/*required*/, False /*expression language supported*/, False /*sensitive*/, 1 
/*property type code*/)
+  processor.addProperty("property name", "description", "default value", True 
/*required*/, False /*expression language supported*/, False /*sensitive*/, 1 
/*property type code*/, None /*controller service type name*/)

Review Comment:
   These `/**/` style comments are not valid python. I'm assuming this comes 
from the NiFi Python API. Otherwise I'd consider changing to keyword arguments.



-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[PR] MINIFICPP-2295 Add controller service support for NiFi python processors [nifi-minifi-cpp]

2024-04-19 Thread via GitHub


lordgamez opened a new pull request, #1762:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1762

   https://issues.apache.org/jira/browse/MINIFICPP-2295
   
   -
   Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
   
   - [ ] Does your PR title start with MINIFICPP- where  is the JIRA 
number you are trying to resolve? Pay particular attention to the hyphen "-" 
character.
   
   - [ ] Has your PR been rebased against the latest commit within the target 
branch (typically main)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [ ] If adding new dependencies to the code, are these dependencies 
licensed in a way that is compatible for inclusion under [ASF 
2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the LICENSE file?
   - [ ] If applicable, have you updated the NOTICE file?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which 
it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check GitHub Actions CI 
results for build issues and submit an update to your PR as soon as possible.
   


-- 
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: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org