exceptionfactory commented on PR #9395:
URL: https://github.com/apache/nifi/pull/9395#issuecomment-2419300387

   > Thanks for the review @exceptionfactory. Unfortunately the catalog can't 
be closed within the `loadTable` method because the jdbc connection is used in 
later part of the processor when the commit happens to the table (this is used 
via the table object). The finally block could be added to the lower try-catch 
block so the `Catalog` type could be kept but the `loadCatalog` extraction 
would be still needed to make the catalog object accessible.
   > 
   > Currently every supported `Catalog` implementation extended from 
`BaseMetastoreCatalog` but if there will be other catalog support added in the 
future that is not extended from that class e.g. RESTCatalog, then your 
suggested solution will be needed (checking if the `Catalog` instance 
implements `Closeable` and handling that in a finally block) Let me know what 
you think.
   
   Thanks for the reply @mark-bathori. With the growing popularity of REST 
Catalog implementations, particularly Apache Polaris, I expect this feature to 
be requested in the future. It may also require more significant refactoring, 
but I think keeping it in mind, in this particular change, is worth doing, so I 
recommend avoiding the switch to `BaseMetastoreCatalog`.


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