exceptionfactory commented on PR #10136: URL: https://github.com/apache/nifi/pull/10136#issuecomment-3127775594
Thanks for the reply @tpalfy. I agree with you that error handling should be more robust, and should attempt to mitigate potential user error. It is important to note, however, that the `DBCPConnectionPoolLookup` makes `database.name` an explicit part of the usage contract. This is unfortunate, but it is an existing implementation problem. Part of the problem with the current situation is that the `getFlowFileFilter()` method never should have been added to the `DBCPService` interface, as it leads to the confusion of responsibility currently in place for `PutSQL`. I think we should separately look at deprecating this method for removal. To help move things in the right direction, one option is to implement checking for the `database.name` attribute in `PutSQL` itself. In the `pollFlowFiles()` method, if the `DBCPService` returns a non-null instance of the `FlowFileFilter`, then there could be two calls to `session.get()`. The first call would use the provided filter. The second call would find FlowFiles missing the `database.name` attribute, and when found, those FlowFiles could be routed to failure. This approach clarifies the tight coupling that already exists with the implementation in `DBCPConnectionPoolLookup`, and also moves in the direction of subsequent deprecation for removal. I'm open to other alternatives that also help move things in this direction, but I think the exception handling approach is less clear, and doesn't move things in the best direction. -- 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]
