jsancio commented on a change in pull request #10431:
URL: https://github.com/apache/kafka/pull/10431#discussion_r628554771
##########
File path:
raft/src/main/java/org/apache/kafka/snapshot/FileRawSnapshotReader.java
##########
@@ -54,8 +54,12 @@ public Records records() {
}
@Override
- public void close() throws IOException {
- fileRecords.close();
+ public void close() {
Review comment:
Yeah. For consistency in the API. For example, most of the methods in
this type perform an IO operation which can throw an `IOException` yet they
have already been implemented to wrap them in a uncheck exception. In some
places we wrap this on a unchecked exception is some places we don't. For the
raft module, we have made an implicit decision to prefer uncheck exceptions for
IO errors.
I tried to clean this up in https://github.com/apache/kafka/pull/10085:
> 5. Removed throws IOException from some methods. Some of types were
inconsistently throwing IOException in some cases and throwing
RuntimeException(..., new IOException(...)) in others. This PR improves the
consistent by wrapping IOException in RuntimeException in a few more places and
replacing Closeable with AutoCloseable.
This PR expands on that work/decision.
--
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:
us...@infra.apache.org
