mcgilman commented on code in PR #10736:
URL: https://github.com/apache/nifi/pull/10736#discussion_r2673942147
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/controller/ControllerFacadeTest.java:
##########
@@ -133,6 +133,9 @@ public void
testSearchConnectorWithNullTermDoesNotInvokeSearchService() {
public void testGetConnectorDefinitionReturnsDefinitionWhenFound() {
final ConnectorDefinition expectedDefinition = new
ConnectorDefinition();
expectedDefinition.setType(TEST_CONNECTOR_TYPE);
+ expectedDefinition.setArtifact(TEST_ARTIFACT);
+ expectedDefinition.setGroup(TEST_GROUP);
+ expectedDefinition.setVersion(TEST_VERSION);
Review Comment:
If you rebase against current NIFI-15258 I think this was already fixed and
these can be removed.
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/StandardConnectorValidationTrigger.java:
##########
@@ -42,10 +43,23 @@ public StandardConnectorValidationTrigger(final
ExecutorService threadPool, fina
public void triggerAsync(final ConnectorNode connector) {
if (!flowInitialized.getAsBoolean()) {
logger.debug("Triggered to perform validation on {} asynchronously
but flow is not yet initialized so will ignore validation", connector);
+ threadPool.schedule(() -> triggerAsync(connector), 1,
TimeUnit.SECONDS);
return;
}
- threadPool.submit(() -> trigger(connector));
+ threadPool.submit(() -> {
+ try {
+ if (connector.isValidationPaused()) {
+ logger.debug("Connector {} is currently marked as having
validation paused; will retry in 1 second", connector);
+ threadPool.schedule(() -> triggerAsync(connector), 1,
TimeUnit.SECONDS);
+ }
+
+ trigger(connector);
+ } catch (final Exception e) {
+ logger.error("Validation for connector {} failed; will retry
in 5 seconds", connector, e);
+ threadPool.schedule(() -> triggerAsync(connector), 5,
TimeUnit.SECONDS);
+ }
+ });
}
@Override
Review Comment:
This is the closest I could comment...
Should `performValidation` only be invoked if validation is necessary?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/StandardConnectorValidationTrigger.java:
##########
@@ -42,10 +43,23 @@ public StandardConnectorValidationTrigger(final
ExecutorService threadPool, fina
public void triggerAsync(final ConnectorNode connector) {
if (!flowInitialized.getAsBoolean()) {
logger.debug("Triggered to perform validation on {} asynchronously
but flow is not yet initialized so will ignore validation", connector);
+ threadPool.schedule(() -> triggerAsync(connector), 1,
TimeUnit.SECONDS);
return;
}
- threadPool.submit(() -> trigger(connector));
+ threadPool.submit(() -> {
+ try {
+ if (connector.isValidationPaused()) {
+ logger.debug("Connector {} is currently marked as having
validation paused; will retry in 1 second", connector);
+ threadPool.schedule(() -> triggerAsync(connector), 1,
TimeUnit.SECONDS);
Review Comment:
When validation is paused, we schedule a retry but the code continues and is
still triggered? Should there be a `return` here?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/StandardConnectorValidationTrigger.java:
##########
@@ -42,10 +43,23 @@ public StandardConnectorValidationTrigger(final
ExecutorService threadPool, fina
public void triggerAsync(final ConnectorNode connector) {
if (!flowInitialized.getAsBoolean()) {
logger.debug("Triggered to perform validation on {} asynchronously
but flow is not yet initialized so will ignore validation", connector);
+ threadPool.schedule(() -> triggerAsync(connector), 1,
TimeUnit.SECONDS);
return;
}
- threadPool.submit(() -> trigger(connector));
+ threadPool.submit(() -> {
+ try {
+ if (connector.isValidationPaused()) {
+ logger.debug("Connector {} is currently marked as having
validation paused; will retry in 1 second", connector);
+ threadPool.schedule(() -> triggerAsync(connector), 1,
TimeUnit.SECONDS);
Review Comment:
Also, there is no deduplication so it may be possible for multiple tasks to
accrue here. Should we try to avoid scheduling them or cancel other pending
tasks that are duplicative?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/components/connector/StandardConnectorValidationTrigger.java:
##########
@@ -30,10 +31,10 @@
public class StandardConnectorValidationTrigger implements
ConnectorValidationTrigger {
private static final Logger logger =
LoggerFactory.getLogger(StandardConnectorValidationTrigger.class);
- private final ExecutorService threadPool;
+ private final ScheduledExecutorService threadPool;
Review Comment:
The javadoc for this class still reference `ExecutorService`, do we care to
keep it aligned here?
--
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]
