chore: Improve S3WritableByteChannel coverage, behaviour, maintenance
#296
Conversation
…-with-resources / Set.of / static import
… invoked with option `CREATE_NEW` should throw FileAlreadyExistsException`
…uctor is invoked without option `CREATE_NEW` nor `CREATE` should throw NoSuchFileException`
…can be instantiated when file exists? and open options are`
…emove CREATE_NEW. Avoid exception handling
…nsible for downloading/uploading
… while uploading`
…ile / write to new file
| @@ -1,13 +1,7 @@ | |||
| package software.amazon.nio.spi.s3; | |||
|
|
|||
| import org.slf4j.Logger; | |||
| import org.checkerframework.checker.nullness.qual.NonNull; | |||
There was a problem hiding this comment.
Not needed for this PR but I would like to add a checker/ nullness annotations to the library. Can we add checker to the gradle buid?
There was a problem hiding this comment.
Sure! The annotations are already present (via some transitive dependency). NonNull is also used in ReadAheadByteChannel if I recall correctly. Do you mind creating an issue for it? It'd be nice to know whether it is just adding the dependency explicitly or integrate the checks on the build via the checkers plugin (and what checks to include)
|
|
||
| import static java.nio.file.StandardOpenOption.CREATE; | ||
| import static java.nio.file.StandardOpenOption.READ; | ||
| import static java.nio.file.StandardOpenOption.WRITE; |
There was a problem hiding this comment.
Another todo (for another PR) would be to add a style template so we are consistent about import order, format etc. Currently I am just using IDEAs defaults but a common template would be great.
There was a problem hiding this comment.
Yes, I agree! Do you have any library already in mind?
Description of changes:
This PR includes several unit tests for
S3WritableByteChannelandS3TransferUtil(new class), as well as some integration tests that useFiles.write. In order to ease testing and complexity, the download / upload behaviour fromS3WritableByteChannelhas been delegated toS3TransferUtil.Additionally, now when
S3WritableByteChannelisclose()d, the temporary file is attempted to be deleted. This should help when writing multiple large files (disk space is freed after each file is copied).By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.