APEXMALHAR-2226 Fixed the Not supported exception while re-deploying the AbstractFileOutput Operator #401
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added my feedback for the changes. Otherwise looks OK.
@@ -636,7 +638,7 @@ protected FSDataOutputStream openStream(Path filepath, boolean append) throws IO | |||
{ | |||
FSDataOutputStream fsOutput; | |||
if (append) { | |||
fsOutput = fs.append(filepath); | |||
return openStreamForNonAppendFS(filepath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
fsOutput = openStreamForNonAppendFS(filepath);
to keep it consistent with the original one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay.
try { | ||
fsOutput = fs.append(filepath); | ||
} catch (IOException e) { | ||
if (e.getMessage().contains("Not supported")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is output of e.getMessage() in this case? Can we make it e.getMessage().equals instead of contains?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Error message consists of very huge contents. I think contains() is the best option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, my bad. Error message contains "Not Supported" only. I changed to equals() instead of contains.
FSDataOutputStream fsOut = fs.create(filepath); | ||
IOUtils.copy(fsIn, fsOut); | ||
flush(fsOut); | ||
fs.delete(appendTmpFile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to close fsIn before delete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. We need to close stream before delete.
ce7e574
to
a58a976
Compare
@yogidevendra Incorporated review comments and Updated the branch. |
Looks OK to me. Would wait for 1 day for any other comments from community else merge it. |
…the AbstractFileOutput Operator
a58a976
to
1333910
Compare
No description provided.