markap14 commented on code in PR #11175:
URL: https://github.com/apache/nifi/pull/11175#discussion_r3389663801
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/dao/impl/StandardConnectorDAO.java:
##########
@@ -157,6 +158,34 @@ public void verifyCancelDrainFlowFile(final String id) {
connector.verifyCancelDrainFlowFiles();
}
+ @Override
+ public void verifyEnterTroubleshooting(final String id) {
+ final ConnectorNode connector = getConnector(id);
+ getConnectorRepository().verifyEnterTroubleshooting(connector);
+ }
+
+ @Override
+ public void enterTroubleshooting(final String id) {
+ final ConnectorNode connector = getConnector(id);
+ getConnectorRepository().enterTroubleshooting(connector);
+ }
+
+ @Override
+ public void verifyEndTroubleshooting(final String id) {
+ final ConnectorNode connector = getConnector(id);
+ getConnectorRepository().verifyEndTroubleshooting(connector);
+ }
+
+ @Override
+ public void endTroubleshooting(final String id) {
+ final ConnectorNode connector = getConnector(id);
+ try {
+ getConnectorRepository().endTroubleshooting(connector);
+ } catch (final FlowUpdateException e) {
+ throw new IllegalStateException("Failed to exit troubleshooting
mode for Connector " + id + ": " + e, e);
+ }
+ }
+
Review Comment:
[claude-opus-4.6] Good catch — you're right, these should use `LOCAL_ONLY`
like the other mutation methods in this class. There's no reason to sync with
the provider before entering or exiting troubleshooting. Fixed in 49cb599: all
four troubleshooting methods now use `requireConnector(id,
ConnectorSyncMode.LOCAL_ONLY)`. Also took the opportunity to fix the `+ e` → `+
e.getMessage()` in the `endTroubleshooting` error message per the earlier
review feedback.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java:
##########
@@ -3704,7 +3734,8 @@ public ConnectorEntity updateConnector(final Revision
revision, final ConnectorD
@Override
public void verifyDeleteConnector(final String id) {
- // For now, DAO will enforce state; expose hook for symmetry
+ final ConnectorNode connector = connectorDAO.getConnector(id);
+ connector.verifyCanDelete();
Review Comment:
[claude-opus-4.6] Agreed — `verifyDeleteConnector` was bypassing the
DAO/Repository path and going directly to the node, which means a
`ConnectorConfigurationProvider` wouldn't be consulted during the pre-check
phase. Fixed in the same commit (49cb599): added `verifyDelete(ConnectorNode)`
to `ConnectorRepository`, `verifyDelete(String)` to `ConnectorDAO`, and routed
`StandardNiFiServiceFacade.verifyDeleteConnector` through the DAO. The
repository implementation currently delegates to `connector.verifyCanDelete()`,
but this provides the hook point for provider consultation in the future,
consistent with the `verifyCreate` pattern.
--
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]
