rdblue commented on issue #1904: URL: https://github.com/apache/iceberg/issues/1904#issuecomment-743432878
@jacques-n, I think this is a bit simpler case. The problem here is that cleanup is being done in `finally` to avoid modifying the throwables that might be caught, and to ensure that the cleanup is always done, even if the `catch` cases change over time. Catching `Exception` means that it is annoyingly difficult thanks to checked exceptions to throw the exception that was caught. The PR I opened solves that problem directly. I agree with the idea of using `AutoCloseable` and multiple try-with-resources, but this problem doesn't need multiple blocks. It looks like `RollbackCloseable` does something very similar to the pattern with the `threw` boolean, except that you'd normally use a separate try, which with the `runSafely` method isn't really needed. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
