ncover21 commented on PR #11240:
URL: https://github.com/apache/nifi/pull/11240#issuecomment-5026969082
Following on from the `isModified` fix, there looks to be a second gap on
the connector migration path: the migrated configuration is committed in memory
but never persisted through the `ConnectorConfigurationProvider`, so a
persistence-backed provider silently reverts the migration on its next sync.
The normal configuration-apply path persists via the provider:
```java
// StandardConnectorRepository#configureConnector
final ConnectorWorkingConfiguration mergedConfiguration =
buildMergedWorkingConfiguration(connector, stepName, configuration);
configurationProvider.save(connector.getIdentifier(), mergedConfiguration);
// persisted
```
The migration commit path does not. After
`StandardConnectorMigrationManager` commits the merged configuration in memory
it only signals completion:
```java
// StandardConnectorMigrationManager
connector.commitMigratedConfiguration(mergedConfiguration); //
in-memory / flow only
...
flowController.getConnectorRepository().notifyMigrationComplete(connectorId,
processGroupId);
// StandardConnectorRepository#notifyMigrationComplete
if (configurationProvider != null) {
configurationProvider.migrationComplete(connectorId,
sourceProcessGroupId);
}
// ConnectorConfigurationProvider
default void migrationComplete(String connectorId, String
sourceProcessGroupId) { } // no-op
```
`save(...)` is never called on the migration path, so a provider that
persists connector configuration never records the migrated result. On the next
`getSyncDirective(...)` / sync the provider rebuilds the working configuration
from its backing store, which still holds the pre-migration configuration, so
the migration is silently reverted and a client reading the configuration back
sees the original (empty) values even though the flow was migrated correctly in
memory.
This is invisible when no provider is configured (the in-memory committed
configuration is authoritative and nothing re-syncs over it), which is likely
why it is not caught by the existing tests.
A fix would be to persist the merged configuration on the migration commit
path, mirroring the apply path, e.g. `configurationProvider.save(connectorId,
buildWorkingConfiguration(connector))` from `notifyMigrationComplete`, so a
persistence-backed provider stores the migrated configuration.
--
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]
