You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/26 00:19:59 UTC
[GitHub] [iceberg] danielcweeks opened a new pull request, #5640: Restore RuntimeIOException for use
danielcweeks opened a new pull request, #5640:
URL: https://github.com/apache/iceberg/pull/5640
I'd like to open this proposal to restore `RuntimeIOException` for continued use and remove the deprecation.
This was originally marked deprecated when java introduced the `UncheckedIOException` which initially seemed like a more standard replacement, but there are a few issues.
1. UncheckedIOException requires a `cause`, which is not present in a number of usages of RuntimeIOException. This complicates the code or requires us to change the exception handling. For example:
```java
if (!file.getParentFile().isDirectory() && !file.getParentFile().mkdirs()) {
throw new RuntimeIOException(
"Failed to create the file's directory at %s.", file.getParentFile().getAbsolutePath());
}
```
becomes
```java
if (!file.getParentFile().isDirectory() && !file.getParentFile().mkdirs()) {
throw new UncheckedIOException(new IOException(
String.format("Failed to create the file's directory at %s.", file.getParentFile().getAbsolutePath())));
}
```
2. `RuntimeIOException` also allows for string formatting which is used extensively in creating messages (also seen in the example above.
3. This class is used pervasively, so there there's a lot of change to deprecate, but `RuntimeIOException` already extends `UncheckedIOException`, so there is very little practical improvement.
### Alternatives:
Another option for deprecation would be to remove the constructors for `RuntimeIOException` that take a cause and update any existing usages that actually have an IO cause to use `UncheckedIOException`. This would leave `RuntimeIOException as a thin wrapper for cases where we want to throw IOException without an originating cause to keep those usages clean
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ajantha-bhat commented on pull request #5640: Restore RuntimeIOException for use
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on PR #5640:
URL: https://github.com/apache/iceberg/pull/5640#issuecomment-1239616536
Just linking the opposite work PR here
https://github.com/apache/iceberg/pull/4776
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Restore RuntimeIOException for use [iceberg]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #5640:
URL: https://github.com/apache/iceberg/pull/5640
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #5640: Restore RuntimeIOException for use
Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #5640:
URL: https://github.com/apache/iceberg/pull/5640#issuecomment-1228666772
I think this is a reasonable way to handle the issue, although I would like to be consistent with the exceptions that are thrown from Iceberg. If we keep `RuntimeIOException`, then we should throw it everywhere rather than throwing `UncheckedIOException`.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] RussellSpitzer commented on pull request #5640: Restore RuntimeIOException for use
Posted by GitBox <gi...@apache.org>.
RussellSpitzer commented on PR #5640:
URL: https://github.com/apache/iceberg/pull/5640#issuecomment-1239642096
I have no problems with keeping RuntimeIOException for all the cases where we don't have an underlying cause. Let's just try to be careful in the future that we don't actually swallow causes which I think as one of our original impetuses for not allowing RuntimeIOExceptions.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org