showuon commented on a change in pull request #10749:
URL: https://github.com/apache/kafka/pull/10749#discussion_r638422416



##########
File path: raft/src/main/java/org/apache/kafka/raft/FileBasedStateStore.java
##########
@@ -91,14 +92,22 @@ private QuorumStateData readStateFromFile(File file) throws 
IOException {
 
             final short dataVersion = dataVersionNode.shortValue();
             return QuorumStateDataJsonConverter.read(dataObject, dataVersion);
+        } catch (IOException e) {
+            throw new UncheckedIOException(
+                    String.format(
+                            "Read the Quorum status exception from the file 
%s",
+                            file
+                    ),
+                    e

Review comment:
       Also, could you re-phrase the error message, ex: `Error while reading 
the Quorum status from the file`
   
   Same comments to the other places.

##########
File path: raft/src/main/java/org/apache/kafka/raft/FileBasedStateStore.java
##########
@@ -91,14 +92,22 @@ private QuorumStateData readStateFromFile(File file) throws 
IOException {
 
             final short dataVersion = dataVersionNode.shortValue();
             return QuorumStateDataJsonConverter.read(dataObject, dataVersion);
+        } catch (IOException e) {
+            throw new UncheckedIOException(
+                    String.format(
+                            "Read the Quorum status exception from the file 
%s",
+                            file
+                    ),
+                    e

Review comment:
       The indent is not consistent with others. Also, should we break the 
exception into 6 lines? I think this could be enough:
   ```java
       throw new UncheckedIOException(
               String.format("Read the Quorum status exception from the file 
%s", file), e);
   ```
   
   Same comments to other places.

##########
File path: raft/src/main/java/org/apache/kafka/raft/FileBasedStateStore.java
##########
@@ -67,7 +68,7 @@ public FileBasedStateStore(final File stateFile) {
         this.stateFile = stateFile;
     }
 
-    private QuorumStateData readStateFromFile(File file) throws IOException {
+    private QuorumStateData readStateFromFile(File file) {

Review comment:
       Should we replace `throws IOException` with `throws 
UncheckedIOException` here?
   
   Same comments to other places.




-- 
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


Reply via email to