yyanyy commented on pull request #2878: URL: https://github.com/apache/iceberg/pull/2878#issuecomment-900663109
Sorry for the late response and thank you for all the feedback @rymurr @kbendick @rdblue ! The use case we have of closing a catalog/FileIO is to handle multi-tenant scenario with Java API interactions to the library, as when a catalog is initiated, all the clients and configurations are for this specific user, and when a new user arrives with a new set of credential and configuration, we need to initiate a new catalog object. Thus we would like to close the catalog and underlying clients to avoid resource leaks if possible. But I agree that when the credentials are static throughout the lifecycle of the application (which is majority of the cases especially when the interaction occurs through engines like Spark), explicitly calling `close()` in either places is not required. The reason for changing catalogs to `AutoCloseable` is because unfortunately the AWS SDK clients extends AutoCloseable which throws `Exception` in method signature, and we need to wrap it with `IOException` if the catalog extends `Closeable` even if we handle idempotency ourselves, when we use a util class to close multiple resources. But since Iceberg uses `Closeable` consistently, I'm happy to do the wrapping. I should be able to update the PR today. Thanks again for reviewing! -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
