markap14 commented on a change in pull request #5116:
URL: https://github.com/apache/nifi/pull/5116#discussion_r648313153
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/processors/script/ExecuteScript.java
##########
@@ -253,31 +226,16 @@ public void onTrigger(ProcessContext context,
ProcessSessionFactory sessionFacto
}
}
- scriptEngine.setBindings(bindings, ScriptContext.ENGINE_SCOPE);
-
- // Execute any engine-specific configuration before the script
is evaluated
- ScriptEngineConfigurator configurator =
-
scriptingComponentHelper.scriptEngineConfiguratorMap.get(scriptingComponentHelper.getScriptEngineName().toLowerCase());
-
- // Evaluate the script with the configurator (if it exists) or
the engine
- if (configurator != null) {
- configurator.init(scriptEngine, scriptToRun,
scriptingComponentHelper.getModules());
- configurator.eval(scriptEngine, scriptToRun,
scriptingComponentHelper.getModules());
- } else {
- scriptEngine.eval(scriptToRun);
- }
+ scriptRunner.run(bindings);
// Commit this session for the user. This plus the outermost
catch statement mimics the behavior
// of AbstractProcessor. This class doesn't extend
AbstractProcessor in order to share a base
// class with InvokeScriptedProcessor
session.commitAsync();
+ scriptingComponentHelper.scriptRunnerQ.offer(scriptRunner);
} catch (ScriptException e) {
- // Reset the configurator on error, this can indicate to the
configurator to recompile the script on next init()
- ScriptEngineConfigurator configurator =
-
scriptingComponentHelper.scriptEngineConfiguratorMap.get(scriptingComponentHelper.getScriptEngineName().toLowerCase());
- if (configurator != null) {
- configurator.reset();
- }
+ // Create a new ScriptRunner to replace the one that caused an
exception
+ scriptingComponentHelper.setupScriptRunners(false, 1,
scriptToRun, getLogger());
Review comment:
Similar to what @pgyori was mentioning above - we need to ensure that we
always account for the script runner. It makes sense to have it offer in the
'good path' and do this in the bad path. But this is done only for
`ScriptException`. It needs to be done for any Exception. It is very possible,
for example, that the script doesn't properly account for all FlowFiles and
session.commitAsync() could then throw an RuntimeException. In that situation,
you'd lose the script runner and after X times (where X = number of concurrent
tasks) the processor would just do nothing, constantly getting nothing from the
poll() call
##########
File path:
nifi-nar-bundles/nifi-scripting-bundle/nifi-scripting-processors/src/main/java/org/apache/nifi/script/ScriptRunnerFactory.java
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.script;
+
+import org.apache.nifi.logging.ComponentLog;
+import org.apache.nifi.processors.script.ScriptRunner;
+import org.apache.nifi.script.impl.ClojureScriptRunner;
+import org.apache.nifi.script.impl.GenericScriptRunner;
+import org.apache.nifi.script.impl.GroovyScriptRunner;
+import org.apache.nifi.script.impl.JavascriptScriptRunner;
+import org.apache.nifi.script.impl.JythonScriptRunner;
+
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineFactory;
+import javax.script.ScriptException;
+import java.io.File;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.LinkedList;
+import java.util.List;
+
+public class ScriptRunnerFactory {
+
+ private final static ScriptRunnerFactory INSTANCE = new
ScriptRunnerFactory();
+
+ private ScriptRunnerFactory() {
+
+ }
+
+ public static ScriptRunnerFactory getInstance() {
+ return INSTANCE;
+ }
+
+ public ScriptRunner createScriptRunner(ScriptEngineFactory
scriptEngineFactory, String scriptToRun, String[] modulePaths)
+ throws ScriptException {
+ ScriptEngine scriptEngine = scriptEngineFactory.getScriptEngine();
+ String scriptEngineName = scriptEngineFactory.getLanguageName();
+ if ("Groovy".equals(scriptEngineName)) {
+ return new GroovyScriptRunner(scriptEngine, scriptToRun, null);
+ }
+ if ("python".equals(scriptEngineName)) {
+ return new JythonScriptRunner(scriptEngine, scriptToRun,
modulePaths);
+ }
+ if ("Clojure".equals(scriptEngineName)) {
+ return new ClojureScriptRunner(scriptEngine, scriptToRun, null);
+ }
+ if ("ECMAScript".equals(scriptEngineName)) {
+ return new JavascriptScriptRunner(scriptEngine, scriptToRun, null);
+ }
+ return new GenericScriptRunner(scriptEngine, scriptToRun, null);
+ }
+
+ /**
+ * Scans the given module paths for JARs. The path itself (whether a
directory or file) will be added to the list
+ * of returned module URLs, and if a directory is specified, it is scanned
for JAR files (files ending with .jar).
+ * Any JAR files found are added to the list of module URLs. This is a
convenience method for adding directories
+ * full of JAR files to an ExecuteScript or InvokeScriptedProcessor
instance, rather than having to enumerate each
+ * JAR's URL.
+ *
+ * @param modulePaths An array of module paths to scan/add
+ * @param log A logger for the calling component, to provide
feedback for missing files, e.g.
+ * @return An array of URLs corresponding to all modules determined from
the input set of module paths.
+ */
+ public URL[] getModuleURLsForClasspath(String scriptEngineName, String[]
modulePaths, ComponentLog log) {
+
+ if ("Clojure".equals(scriptEngineName)
+ || "Groovy".equals(scriptEngineName)
+ || "ECMAScript".equals(scriptEngineName)) {
+
+ List<URL> additionalClasspath = new LinkedList<>();
+ if (modulePaths != null) {
Review comment:
May be worth refactoring to use the "fail-fast" / "escape-fast" approach
here:
```
if (modulesPaths == null) {
return new URL[0];
}
```
And so forth. Should result in minimal nesting and easier-to-read code IMO
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
