ORC-301. extractFileTail should open a file in try statement
#218
Conversation
dongjoon-hyun
changed the title
extractFileTail should open a file in try statementextractFileTail should open a file in try statement
|
Hi, @omalley and @prasanthj . |
| OrcProto.PostScript ps; | ||
| OrcProto.FileTail.Builder fileTailBuilder = OrcProto.FileTail.newBuilder(); | ||
| long modificationTime; | ||
| try { |
There was a problem hiding this comment.
Maybe replace with try-with-resource block?
There was a problem hiding this comment.
Thank you for review, @prasanthj .
Ya. I thought to use try-with-resource first, but it needs more change to print out the log at Line 610.
There was a problem hiding this comment.
I think this is the minimal change while keeping the current behavior.
There was a problem hiding this comment.
Line 610 doesn't add much value as close() will throw IOException anyway which should be sufficient to know the reason.
There was a problem hiding this comment.
So, you mean it's okay to remove the following in this PR, right?
catch (IOException ex) {
LOG.error("Failed to close the file after another error", ex);
}
|
Thank you, @prasanthj . I updated according to your advice. |
|
lgtm, +1 |
|
Thank you so much, @prasanthj ! |
Fixes #218 Signed-off-by: Prasanth Jayachandran <prasanthj@apache.org>
|
Thank you for merging, @prasanthj ! |
2 participants
This PR moves the file open position to prevent opened file leakage more robustly.