NissimShiman commented on code in PR #6254:
URL: https://github.com/apache/nifi/pull/6254#discussion_r982906854
##########
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/test/java/org/apache/nifi/processors/script/TestExecuteJython.java:
##########
@@ -61,6 +67,83 @@ public void
testReadFlowFileContentAndStoreInFlowFileAttributeWithScriptBody() t
result.get(0).assertAttributeEquals("from-content", "test content");
}
+ /**
+ * Tests a Jython script that references an outside python module
+ *
+ */
+ @Test
+ public void testAccessModuleAndStoreInFlowFileAttributeWithScriptBody() {
+ runner.setValidateExpressionUsage(false);
+
runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"python");
+ runner.setProperty(ScriptingComponentUtils.MODULES,
"target/test/resources/jython/");
+ runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY,
+ "from org.apache.nifi.processors.script import ExecuteScript\n"
+ + "from test_external_module import ExternalModule\n"
+ + "externalModule = ExternalModule()\n"
+ + "flowFile = session.get()\n"
+ + "flowFile = session.putAttribute(flowFile, \"key\",
externalModule.testHelloWorld())\n"
+ + "session.transfer(flowFile,
ExecuteScript.REL_SUCCESS)");
+
+ runner.assertValid();
+ runner.enqueue("test content".getBytes(StandardCharsets.UTF_8));
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(ExecuteScript.REL_SUCCESS, 1);
+ final List<MockFlowFile> result =
runner.getFlowFilesForRelationship(ExecuteScript.REL_SUCCESS);
+ result.get(0).assertAttributeEquals("key", "helloWorld");
+ }
+
+
+ /**
+ * Tests a Jython script when adding comma separated list of all existing
directories from PYTHONPATH
+ * as value for {@link ScriptingComponentUtils} MODULES property.
+ *
+ * @throws Exception If PYTHONPATH cannot be retrieved.
+ */
+ @Test
+ @EnabledOnOs(OS.LINUX)
+ public void
testAccessPythonPackageModulesAndStoreInFlowFileAttributeWithScriptBody()
throws Exception {
+ String attributeName = "key";
+ String attributeValue = "helloWorld";
+ runner.setValidateExpressionUsage(false);
+
runner.setProperty(scriptingComponent.getScriptingComponentHelper().SCRIPT_ENGINE,
"python");
+ runner.setProperty(ScriptingComponentUtils.MODULES,
+ String.join(",", getExistingPythonPathModuleDirectories()));
+ runner.setProperty(ScriptingComponentUtils.SCRIPT_BODY,
+ "from org.apache.nifi.processors.script import ExecuteScript\n"
+ + "flowFile = session.get()\n"
+ + "flowFile = session.putAttribute(flowFile," + "\""
+ attributeName + "\", '" + attributeValue + "')\n"
+ + "session.transfer(flowFile,
ExecuteScript.REL_SUCCESS)");
+
+ runner.assertValid();
+ runner.enqueue("test content".getBytes(StandardCharsets.UTF_8));
+ runner.run();
+
+ runner.assertAllFlowFilesTransferred(ExecuteScript.REL_SUCCESS, 1);
+ final List<MockFlowFile> result =
runner.getFlowFilesForRelationship(ExecuteScript.REL_SUCCESS);
+ result.get(0).assertAttributeEquals(attributeName, attributeValue);
+ }
+
+ /**
+ * Method which retrieves the PYTHONPATH and builds a java.util.List of
existing directories on the PYTHONPATH.
+ * This method uses java.lang.ProcessBuilder and java.lang.Process to
execute python to obtain the PYTHONPATH.
+ *
+ * @return java.util.List of existing directories on PYTHONPATH.
+ * @throws Exception If an error occurs when executing a java.lang.Process.
+ */
+ List<String> getExistingPythonPathModuleDirectories() throws Exception{
Review Comment:
@mattyb149 Thank you very much for looking at this!
I hear you ... this is overkill for a unit test, but I *think* it is the
only way to mimic the second situation mentioned in the jira ticket (i.e. where
a user used the machine's python path , /usr/lib/python3.7/ or whatever) in the
Module Directory property).
This ended up recursively sucking in so many files that the the processor
couldn't function.
Of course, only external modules are supposed to referenced with this
property (as opposed to an entire python installation), so essentially this
test is more to protect against user error as opposed to an actual processor
flaw.
So ... maybe it is overkill to include this to begin with.as it may be
beyond the scope of unit tests to be looking for the set of possible user
errors.
So I think either we let this go or we keep it, as I'm not sure there is a
middle ground in testing that particular use case
Let me know if we wish to drop it.
Thank You!
--
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]
