tpalfy commented on PR #10136:
URL: https://github.com/apache/nifi/pull/10136#issuecomment-3127690701

   @exceptionfactory I can agree with most what you say on a technical level. 
But there are conceptual levels to consider in my opinion.
   
   In general, a robust, professional system is better off when prepared for 
various user errors as well. It's not great when the requirement for 
"database.name" means that if it is missing, the whole flow can get stuck and 
potentially crash a production system. While user errors can lead to that 
either way, NiFi is a perfect candidate for a component to prevent that.
   
   The `ExecuteSQL` doesn't use the FlowFileFilter filter indeed, but that's 
just a technical detail. From a user perspective, they can use it with a 
`DBCPConnectionPoolLookup` and rely on it sending faulty FlowFiles to the 
failure relationship.
   
   If anything, `PutSQL` should definitely do this even more so - when it's 
`Rollback On Failure` is set to `true`, as explicitly promises to do so by the 
documention.
   Having to add another processor _instead_ is hardly a solution post-mortem 
and in general a bad user experience.
   
   I do agree that the `DBCPConnectionPoolLookup.getFlowFileFilter()` should 
have been designed in a way - together with PutSQL - that it doesn't throw an 
exception. But the fact of the matter is, this change has been approved in this 
way - as part of the core mechanic of the implementation. Considering a major 
refactor is hardly a proper cost-benefit analysis result.
   
   I updated the PR so that the adjustment only takes effect when `Rollback On 
Failure` is set to `true` though. This hopefully both limits any backward 
compatibility liability and makes the functionality more in tune with common 
sense user expectation and with the documentation.


-- 
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]

Reply via email to