nifi
nifi copied to clipboard
NIFI-10287 ExecuteScript - Allow python scripts to use external modules
Summary
NIFI-10287 This resolves a bug that crept in where the ExecuteScript processor used to support using external modules for python, but has lost that functionality. This has been fixed in this ticket.
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
- [x] Apache NiFi Jira issue created
Pull Request Tracking
- [x] Pull Request title starts with Apache NiFi Jira issue number, such as
NIFI-00000 - [x] Pull Request commit message starts with Apache NiFi Jira issue number, as such
NIFI-00000
Pull Request Formatting
- [x] Pull Request based on current revision of the
mainbranch - [x] Pull Request refers to a feature branch with one commit containing changes
Verification
Please indicate the verification steps performed prior to pull request creation.
Build
- [x] Build completed using
mvn clean install -P contrib-check- [x] JDK 8
- [x] JDK 11
- [x] JDK 17
Licensing
- [x] New dependencies are compatible with the Apache License 2.0 according to the License Policy
- [x] New dependencies are documented in applicable
LICENSEandNOTICEfiles
Documentation
- [x] Documentation formatting appears as expected in rendered files
LGTM+1
Thank you very much @dan-s1 for the review and for volunteering the unit test for the use case that was missed!
Squashed commits after approval to make it easier for additional reviewers to see code and to capture attribution for @dan-s1's generous contribution.
Unit test looks good. I built the code (including -Pcontrib-check), and all looks good. I tested by instantiating an ExecuteScript processor. It was configured with a module directory with a .py module file similar to the one used in the unit test. Ran data and it worked like a charm. I changed the module code and re-ran a flowfile (without stopping the processor.) The changes were not reflected in the processed flowfile. However, restarting the processor picks up the module update. I believe this is expected behavior since we do not want to reload modules (or even check if they should be reloaded) on every flowfile; I believe that would be overly burdensome.
Overall, looks good to merge to me. Thanks @NissimShiman ! +1
@mattyb149 @exceptionfactory @MikeThomsen I removed the unit test that was concerning so I think we are good. The unit test for the main use case of the ticket remains. Please let me know if there are other holdup related concerns.
@mattyb149 I was wondering if you could take a look at this again. The previous issue you (and another reviewer) mentioned has been resolved.
+1 LGTM, thanks for the improvement and sticking with this! Merging to main
Thank you @mattyb149 !