exceptionfactory commented on code in PR #10455:
URL: https://github.com/apache/nifi/pull/10455#discussion_r2463009214
##########
nifi-extension-bundles/nifi-groovyx-bundle/nifi-groovyx-processors/src/main/java/org/apache/nifi/processors/groovyx/ExecuteGroovyScript.java:
##########
@@ -138,6 +143,8 @@ public class ExecuteGroovyScript extends AbstractProcessor {
public static final Relationship REL_FAILURE = new
Relationship.Builder().name("failure").description("FlowFiles that failed to be
processed").build();
+ public static final int VALIDATION_TIMEOUT_MINUTES = 5;
Review Comment:
This can be a private variable:
```suggestion
private static final int VALIDATION_TIMEOUT_MINUTES = 5;
```
##########
nifi-extension-bundles/nifi-groovyx-bundle/nifi-groovyx-processors/src/main/java/org/apache/nifi/processors/groovyx/ExecuteGroovyScript.java:
##########
@@ -211,13 +217,27 @@ protected Collection<ValidationResult>
customValidate(final ValidationContext co
this.addClasspath =
context.getProperty(ADD_CLASSPATH).evaluateAttributeExpressions().getValue();
//ADD_CLASSPATH
this.groovyClasspath =
context.newPropertyValue(GROOVY_CLASSPATH).evaluateAttributeExpressions().getValue();
//evaluated from ${groovy.classes.path} global property
- final Collection<ValidationResult> results = new HashSet<>();
+ String errorText = null;
+ ExecutorService executor = Executors.newSingleThreadExecutor();
try {
- getGroovyScript();
- } catch (Throwable t) {
- results.add(new
ValidationResult.Builder().subject("GroovyScript").input(this.scriptFile !=
null ? this.scriptFile.toString() :
null).valid(false).explanation(t.toString()).build());
+ Future<Script> groovyFuture =
executor.submit(this::getGroovyScript);
+ groovyFuture.get(VALIDATION_TIMEOUT_MINUTES, TimeUnit.MINUTES);
+ } catch (ExecutionException e) {
+ errorText = e.getCause().toString();
+ } catch (Exception e) {
+ errorText = e.toString();
+ } finally {
+ if (!executor.isTerminated()) {
Review Comment:
Is this check necessary? It looks like the executor should be shutdown
regardless.
##########
nifi-extension-bundles/nifi-groovyx-bundle/nifi-groovyx-processors/src/main/java/org/apache/nifi/processors/groovyx/ExecuteGroovyScript.java:
##########
@@ -211,13 +217,27 @@ protected Collection<ValidationResult>
customValidate(final ValidationContext co
this.addClasspath =
context.getProperty(ADD_CLASSPATH).evaluateAttributeExpressions().getValue();
//ADD_CLASSPATH
this.groovyClasspath =
context.newPropertyValue(GROOVY_CLASSPATH).evaluateAttributeExpressions().getValue();
//evaluated from ${groovy.classes.path} global property
- final Collection<ValidationResult> results = new HashSet<>();
+ String errorText = null;
+ ExecutorService executor = Executors.newSingleThreadExecutor();
Review Comment:
I recommend using a Virtual Thread executor to minimize thread creation.
##########
nifi-extension-bundles/nifi-groovyx-bundle/nifi-groovyx-processors/src/main/java/org/apache/nifi/processors/groovyx/ExecuteGroovyScript.java:
##########
@@ -138,6 +143,8 @@ public class ExecuteGroovyScript extends AbstractProcessor {
public static final Relationship REL_FAILURE = new
Relationship.Builder().name("failure").description("FlowFiles that failed to be
processed").build();
+ public static final int VALIDATION_TIMEOUT_MINUTES = 5;
Review Comment:
This looks like fail safe measure, so 5 minutes seems like a reasonable
worst case scenario for now.
--
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]
