adutra commented on PR #13384: URL: https://github.com/apache/iceberg/pull/13384#issuecomment-3005151177
> changes LGTM but wouldn't we have a similar issue in other places where the auth manager is used, such as `VendedAdlsCredentialsProvider`/ `OAuth2RefreshCredentialsHandler`? I think in those two classes the `CloseableGroup` should be instantiated much earlier and managed outside of the `close()` method @nastra I think they are fine? As it stands now, in these classes, every closeable field that is written will get closed when the `close()` method is called. The `CloseableGroup` is just a trick to avoid a bunch of nested try-finally blocks. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
