sfc-gh-mpayne commented on code in PR #10931:
URL: https://github.com/apache/nifi/pull/10931#discussion_r2843608736
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/components/connector/StandardConnectorNode.java:
##########
@@ -309,6 +309,11 @@ private void destroyWorkingContext() {
public void abortUpdate(final Throwable cause) {
stateTransition.setCurrentState(ConnectorState.UPDATE_FAILED);
stateTransition.setDesiredState(ConnectorState.UPDATE_FAILED);
+ validationState.set(new ValidationState(ValidationStatus.INVALID,
List.of(new ValidationResult.Builder()
+ .subject("Flow Update Failure")
+ .valid(false)
+ .explanation("The flow could not be updated.")
+ .build())));
Review Comment:
I don't believe this is the right approach here. Consider this:
- Connector is valid and running.
- User stops Connector
- User decides to reconfigure and applies the update
- Connector needs to drain but it times out waiting for that to occur
- At this point, the prepareForUpdate() would fail. And abortUpdate would
get called. The connector was not updated, but it is still entirely valid and
we should be able to start it.
On the other hand, on startup if an Exception is thrown we don't want it to
cause startup failure.
So I think we may need to update the method definition here to allow a
boolean indicating whether or not this update should make the Connector invalid
- or move the logic into some other method and call it in the right context.
--
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]
