You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "lordgamez (via GitHub)" <gi...@apache.org> on 2025/09/22 12:57:47 UTC

[PR] MINIFICPP-2635 Handle dynamic properties correctly in python processors [nifi-minifi-cpp]

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

   - Fix dynamic property support flag setting in python processors
   - Only support dynamic properties in NiFi style python processors if `getDynamicPropertyDescriptor` is defined
   - Fix `yieldResources` python binding
   - Add `getDynamicProperty` and `getDynamicPropertyKeys` to minifi python API
   - Only require property name and description when adding property to processor using minifi python API
   - Add docker tests for dynamic property handling in python processors
   
   https://issues.apache.org/jira/browse/MINIFICPP-2635
   
   ---------
   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-XXXX where XXXX 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-unsubscribe@nifi.apache.org

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


Re: [PR] MINIFICPP-2635 Handle dynamic properties correctly in python processors [nifi-minifi-cpp]

Posted by "lordgamez (via GitHub)" <gi...@apache.org>.
lordgamez commented on code in PR #2034:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2034#discussion_r2372222211


##########
PROCESSORS.md:
##########
@@ -722,7 +722,7 @@ In the list below, the names of required properties appear in bold. Any other pr
 
 ### Description
 
-Executes a script given the flow file and a process session. The script is responsible for handling the incoming flow file (transfer to SUCCESS or remove, e.g.) as well as any flow files created by the script. If the handling is incomplete or incorrect, the session will be rolled back. Scripts must define an onTrigger function which accepts NiFi Context and ProcessSession objects. Scripts are executed once when the processor is run, then the onTrigger method is called for each incoming flowfile. This enables scripts to keep state if they wish. The python script files are expected to contain `describe(processor)` and `onTrigger(context, session)`.
+DEPRECATED. This processor should only be used internally for running NiFi and MiNiFi C++ style python processors. Do not use this processor in your own flows, move your python processors to the minifi-python directory instead, where they will be parsed, and then they can be used with their filename as the processor class in the flow configuration. This processor executes a script given the flow file and a process session. The script is responsible for handling the incoming flow file (transfer to SUCCESS or remove, e.g.) as well as any flow files created by the script. If the handling is incomplete or incorrect, the session will be rolled back. Scripts must define an onTrigger function which accepts NiFi Context and ProcessSession objects. Scripts are executed once when the processor is run, then the onTrigger method is called for each incoming flowfile. This enables scripts to keep state if they wish. The python script files are expected to contain `describe(processor)` and `onTrig
 ger(context, session)`.

Review Comment:
   Updated in https://github.com/apache/nifi-minifi-cpp/pull/2034/commits/c2050514116f06ba9755e6eec1a5997529936d1f



##########
extensions/python/ExecutePythonProcessor.h:
##########
@@ -105,7 +111,7 @@ class ExecutePythonProcessor : public core::ProcessorImpl {
     return python_properties_;
   }
 
-  bool getPythonSupportDynamicProperties() {
+  bool supportsDynamicProperties() const override {
     return python_dynamic_;
   }

Review Comment:
   Updated in https://github.com/apache/nifi-minifi-cpp/pull/2034/commits/c2050514116f06ba9755e6eec1a5997529936d1f



-- 
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-unsubscribe@nifi.apache.org

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


Re: [PR] MINIFICPP-2635 Handle dynamic properties correctly in python processors [nifi-minifi-cpp]

Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #2034:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2034#discussion_r2371923043


##########
PROCESSORS.md:
##########
@@ -722,7 +722,7 @@ In the list below, the names of required properties appear in bold. Any other pr
 
 ### Description
 
-Executes a script given the flow file and a process session. The script is responsible for handling the incoming flow file (transfer to SUCCESS or remove, e.g.) as well as any flow files created by the script. If the handling is incomplete or incorrect, the session will be rolled back. Scripts must define an onTrigger function which accepts NiFi Context and ProcessSession objects. Scripts are executed once when the processor is run, then the onTrigger method is called for each incoming flowfile. This enables scripts to keep state if they wish. The python script files are expected to contain `describe(processor)` and `onTrigger(context, session)`.
+DEPRECATED. This processor should only be used internally for running NiFi and MiNiFi C++ style python processors. Do not use this processor in your own flows, move your python processors to the minifi-python directory instead, where they will be parsed, and then they can be used with their filename as the processor class in the flow configuration. This processor executes a script given the flow file and a process session. The script is responsible for handling the incoming flow file (transfer to SUCCESS or remove, e.g.) as well as any flow files created by the script. If the handling is incomplete or incorrect, the session will be rolled back. Scripts must define an onTrigger function which accepts NiFi Context and ProcessSession objects. Scripts are executed once when the processor is run, then the onTrigger method is called for each incoming flowfile. This enables scripts to keep state if they wish. The python script files are expected to contain `describe(processor)` and `onTrig
 ger(context, session)`.

Review Comment:
   nitpicking, but maybe put a line break (or two) between the deprecation warning and the legacy description to separate them better visually



##########
extensions/python/ExecutePythonProcessor.h:
##########
@@ -105,7 +111,7 @@ class ExecutePythonProcessor : public core::ProcessorImpl {
     return python_properties_;
   }
 
-  bool getPythonSupportDynamicProperties() {
+  bool supportsDynamicProperties() const override {
     return python_dynamic_;
   }

Review Comment:
   I would move this to just before line 92 and make it single-line; that would make it clearer to someone reading the code why we are not using the full standard `ADD_COMMON_VIRTUAL_FUNCTIONS_FOR_PROCESSORS` set of functions.



-- 
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-unsubscribe@nifi.apache.org

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


Re: [PR] MINIFICPP-2635 Handle dynamic properties correctly in python processors [nifi-minifi-cpp]

Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink closed pull request #2034: MINIFICPP-2635 Handle dynamic properties correctly in python processors
URL: https://github.com/apache/nifi-minifi-cpp/pull/2034


-- 
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-unsubscribe@nifi.apache.org

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