Added UploadDirectoryRequest.Builder#uploadRequestTransformer that al… #2799
Conversation
...java/software/amazon/awssdk/transfer/s3/S3TransferManagerUploadDirectoryIntegrationTest.java
Outdated
Show resolved
Hide resolved
millems
force-pushed
the
millem/transfer-manager-put-object-modifier
branch
from
d53e900 to
b1dfa94
Compare
...java/software/amazon/awssdk/transfer/s3/S3TransferManagerUploadDirectoryIntegrationTest.java
Outdated
Show resolved
Hide resolved
...nsfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/UploadRequestTransformer.java
Outdated
Show resolved
Hide resolved
...ransfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/UploadDirectoryRequest.java
Outdated
Show resolved
Hide resolved
…ransformer that allows modifying the UploadFileRequests generated during directory uploads.
millems
force-pushed
the
millem/transfer-manager-put-object-modifier
branch
from
3def570 to
4dd1454
Compare
| * // Add a LoggingTransferListener to every transfer within the upload directory request | ||
| * TransferRequestOverrideConfiguration fileUploadConfiguration = | ||
| * TransferRequestOverrideConfiguration.builder() | ||
| * .addListener(LoggingTransferListener.create()) |
There was a problem hiding this comment.
Out of scope of this PR but worth noting: This works, but it's going to result in using the same listener for every invocation, when the current LoggingTransferListener implementation assumes that it's not going to be reused. We've so far refrained from adding listeners at the client-level for this reason. A user could implement a reusable listener by internally mapping something like TransferRequest to internal listener state, but this is a lot of complexity. I wonder if we should change listeners to be declared with a Supplier instead.
There was a problem hiding this comment.
I see, yeah, seems like it's easy to make that mistake, then. I certainly thought this would be fine. What do you recommend I do for the purposes of this PR?
There was a problem hiding this comment.
If we moved the LoggingTransferListener.create() call to be inside your lambda, it would create a new one for every request and work as expected, but that may make the code less readable. If we can update the example to do that, without severely affecting legibility, then I would recommend that. Otherwise I think it's okay to use your example as-is and we can do a fast-follow-up on whether to accept a Supplier or not.
There was a problem hiding this comment.
It would unfortuantely be very difficult to make it readable with the lambdas. I'll leave it as-is and we can move it to a supplier asynchronously.
|
SonarCloud Quality Gate failed. |
…lows modifying the UploadRequests generated during directory uploads.