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/05/16 04:13:49 UTC
[GitHub] [iceberg] ajantha-bhat opened a new pull request, #4776: Replace deprecated RuntimeIOException with UncheckedIOException
ajantha-bhat opened a new pull request, #4776:
URL: https://github.com/apache/iceberg/pull/4776
Replace deprecated `RuntimeIOException` with `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] rdblue commented on a diff in pull request #4776: Replace deprecated RuntimeIOException with UncheckedIOException
Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#discussion_r875038397
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -59,9 +59,9 @@ public PositionOutputStream create() {
}
if (!file.getParentFile().isDirectory() && !file.getParentFile().mkdirs()) {
- throw new RuntimeIOException(
+ throw new UncheckedIOException(new IOException(String.format(
Review Comment:
Rather than creating an `IOException`, I think this should throw a different exception.
--
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] Replace deprecated RuntimeIOException with UncheckedIOException [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#issuecomment-1855384162
Closing as we concluded to undo deprecation.
Fixed by 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] ajantha-bhat commented on a diff in pull request #4776: Replace deprecated RuntimeIOException with UncheckedIOException
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#discussion_r931711479
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -59,9 +59,9 @@ public PositionOutputStream create() {
}
if (!file.getParentFile().isDirectory() && !file.getParentFile().mkdirs()) {
- throw new RuntimeIOException(
+ throw new UncheckedIOException(new IOException(String.format(
Review Comment:
@rdblue: As this is needed for 1.0 release. Can you please reply to the above question?
--
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 a diff in pull request #4776: Replace deprecated RuntimeIOException with UncheckedIOException
Posted by GitBox <gi...@apache.org>.
ajantha-bhat commented on code in PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#discussion_r875057628
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -59,9 +59,9 @@ public PositionOutputStream create() {
}
if (!file.getParentFile().isDirectory() && !file.getParentFile().mkdirs()) {
- throw new RuntimeIOException(
+ throw new UncheckedIOException(new IOException(String.format(
Review Comment:
Yeah. Very few places have this. For these cases, shall I replace `UncheckedIOException` with a new exception class similar to the existing `NotFoundException` which extends `RuntimeException`?
--
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] Replace deprecated RuntimeIOException with UncheckedIOException [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat closed pull request #4776: Replace deprecated RuntimeIOException with UncheckedIOException
URL: 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] Replace deprecated RuntimeIOException with UncheckedIOException [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#discussion_r1425558869
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -24,8 +24,8 @@
import java.io.RandomAccessFile;
import java.nio.file.Paths;
import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.FileHandlingException;
Review Comment:
Yeah. Description mentions that this PR depends on #6887
Idea is that, I will rebase this PR after #6887 is merged.
Reviewer can easily review this PR as it will just blind replacement once I rebase.
--
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] Replace deprecated RuntimeIOException with UncheckedIOException [iceberg]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#discussion_r1425380109
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -24,8 +24,8 @@
import java.io.RandomAccessFile;
import java.nio.file.Paths;
import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.FileHandlingException;
Review Comment:
Don't we have an overlap between this PR and #6887 ?
Maybe worth to define an order and gather both PRs in a single PR.
--
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] Replace deprecated RuntimeIOException with UncheckedIOException [iceberg]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on code in PR #4776:
URL: https://github.com/apache/iceberg/pull/4776#discussion_r1425565613
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -24,8 +24,8 @@
import java.io.RandomAccessFile;
import java.nio.file.Paths;
import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.FileHandlingException;
Review Comment:
Awesome, thanks :) It will be easier to review 👍
##########
api/src/main/java/org/apache/iceberg/Files.java:
##########
@@ -24,8 +24,8 @@
import java.io.RandomAccessFile;
import java.nio.file.Paths;
import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.FileHandlingException;
Review Comment:
Awesome, thanks :) It will be easier to review 👍
--
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