bobpaulin commented on PR #11190: URL: https://github.com/apache/nifi/pull/11190#issuecomment-4353408253
Thank you for the quick review @kevdoran! Comments inline > Thanks for this @bobpaulin. > > We should add syncing assets to `getConnector()` in case assets have been added in the external provider since it was last loaded in NiFi. I understand the intent here but the frequency of calling getConnector throughout the framework gives me pause. > > We also need to call `connector.syncAssets(id)` from inside of `buildMergedWorkingConfiguration()` before calling `configurationProvider.load(connectorId)` for the same reason. > This is also called frequently since it's tied into configureConnector(), but less so than getConnector(). I think this could be done. But this depends on how the ConnectorConfigurationProvider handles the save event. If assets may be synced on save I think it's possible this could be avoided. It does not look like this code validates anything prior to the save so as long as it was handled before the method exits I'd expect someone working in the Connector UI would not notice any issue. However I do think this ties in a bit to where we believe the responsibility of the ConnectorRepository vs the ConnectorConfigurationProvider stand. If we do it in the repository all providers MUST sync the assets when saving each step. If we allow the ConnectorConfigurationProvider it's up to the provider implementor. > While not entirely necessary, I would be ok with adding asset syncing to `syncFromProvider()`, as I no longer think we should have different behavior for `syncFromProvider()` and `syncAssetsFromProvider()`. `syncAssetsFromProvider()` could just call `syncFromProvider()` backwards compatibility. The only reason I hesitate to do this is that would sync assets for calls to getConnectors() as well, which is probably not entirely necessary. Making the asset sync always up-to-date might fix a number of ailments but may kill the patient. -- 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]
