feat: Initial Classes and Interface for Transfer Manager#1874
feat: Initial Classes and Interface for Transfer Manager#1874sydney-munro merged 16 commits intofeat/transfer-managerfrom
Conversation
BenWhitehead
left a comment
There was a problem hiding this comment.
A few minor things to change, and a question or two.
google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DownloadJob.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DownloadJob.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DownloadJob.java
Outdated
Show resolved
Hide resolved
| public DownloadResult build() { | ||
| checkNotNull(input); | ||
| checkNotNull(status); | ||
| return new DownloadResult(input, outputDestination, status, exception); |
There was a problem hiding this comment.
TODO: validate either outputDestination or exception are non-null relative to the value of status.
There was a problem hiding this comment.
I will add this in a follow up. I know its only a few lines but was trying to get through the other comments first.
google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DownloadJob.java
Outdated
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DownloadResult.java
Show resolved
Hide resolved
google-cloud-storage/src/main/java/com/google/cloud/storage/transfermanager/DownloadResult.java
Outdated
Show resolved
Hide resolved
...oud-storage/src/main/java/com/google/cloud/storage/transfermanager/ParallelUploadConfig.java
Show resolved
Hide resolved
|
|
||
| class TransferManagerConfig { | ||
| @NonNull private final int maxWorkers; | ||
| @NonNull private final int perWorkerBufferSize; |
There was a problem hiding this comment.
Thoughts on changing this to maxTotalBufferSize which could leave us wiggle room on how much per worker we use if we're able to determine scaling up buffer size can increase throughput.
There was a problem hiding this comment.
I feel like we are moving further from the original design which was chunkSize if we continue in this direction. I don't think this is necessarily a bad thing but node and python are also out already with chunkSize.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.