aaltay commented on a change in pull request #12681:
URL: https://github.com/apache/beam/pull/12681#discussion_r554088688
##########
File path:
sdks/java/extensions/sorter/src/main/java/org/apache/beam/sdk/extensions/sorter/NativeFileSorter.java
##########
@@ -139,6 +139,7 @@ public void add(byte[] key, byte[] value) throws
IOException {
}
} finally {
inputStream.close();
+ Preconditions.checkArgument(dataFile.delete());
Review comment:
It was not obvious to me initially. IIUC, this is safe only because
`sort` could only be called once.
One question for my learning: Is not the primary purpose of Preconditions is
to check input arguments to a function? In this case it is used to assert on
the return value of delete call, and that is not a precondition here. Even
though functionally this does not make a difference. (And a related question,
do we even need to check this? Even if delete fails here that is ok, it is a
temp file and will be cleaned on exit.)
----------------------------------------------------------------
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:
[email protected]
