-
Notifications
You must be signed in to change notification settings - Fork 870
Add multipartupload lifecycle tracking #4061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add multipartupload lifecycle tracking #4061
Conversation
add dev config fix unseekable stream failure initial request update try catch fix null pointer add comment stack-info: PR: #4061, branch: GarrettBeatty/stacked/3
dfea2d8 to
ff96820
Compare
103979e to
7c7699c
Compare
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.
Pull Request Overview
This PR adds multipart upload event lifecycles to the AWS S3 Transfer Utility, enabling tracking of upload initiation, completion, and failure events during multipart uploads.
- Adds lifecycle event firing (initiated, completed, failed) to the multipart upload command
- Implements comprehensive integration tests for all event scenarios including unseekable streams
- Includes proper dev config for patch-level changes
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
MultipartUploadCommand.async.cs |
Adds event firing for transfer lifecycle events during async multipart uploads |
MultipartUploadCommand.cs |
Implements helper methods to fire transfer lifecycle events and updates progress callback signature |
TransferUtilityTests.cs |
Adds comprehensive integration tests for multipart upload lifecycle events |
433a9a6d-b8ea-4676-b763-70711e8288e3.json |
Dev config file specifying patch-level changes with appropriate changelog messages |
| FireTransferFailedEvent(); | ||
|
|
||
| // Can't do async invocation in the catch block, doing cleanup synchronously. | ||
| Cleanup(initResponse.UploadId, pendingUploadPartTasks); |
Copilot
AI
Oct 18, 2025
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.
Potential null reference exception when initResponse is null. The code should check if initResponse is not null before accessing UploadId, or pass initResponse?.UploadId to handle the null case properly.
| Cleanup(initResponse.UploadId, pendingUploadPartTasks); | |
| Cleanup(initResponse?.UploadId, pendingUploadPartTasks); |
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.
not sure if this is needed because initresponse could only be null if the initial request failed and if it did we throw exception above?
7c7699c to
89d3e3d
Compare
89d3e3d to
ffc4c2d
Compare
ffc4c2d to
e2a809c
Compare
e2a809c to
c645a1f
Compare
c645a1f to
59fdbf7
Compare
afd1f00 to
0bd2c46
Compare
0bd2c46 to
23aa6b9
Compare
|
|
||
| var progressArgs = new UploadProgressArgs(e.IncrementTransferred, transferredBytes, this._contentLength, | ||
| e.CompensationForRetry, this._fileTransporterRequest.FilePath); | ||
| e.CompensationForRetry, this._fileTransporterRequest.FilePath, this._fileTransporterRequest); |
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.
we are also adding the request object to the progress args as part of this change
sdk/src/Services/S3/Custom/Transfer/Internal/MultipartUploadCommand.cs
Outdated
Show resolved
Hide resolved
generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json
Outdated
Show resolved
Hide resolved
generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json
Outdated
Show resolved
Hide resolved
generator/.DevConfigs/433a9a6d-b8ea-4676-b763-70711e8288e3.json
Outdated
Show resolved
Hide resolved
23aa6b9 to
d1ba823
Compare
d1ba823 to
cfaa4b4
Compare
cfaa4b4 to
1263ff8
Compare
stack-info: PR: #4061, branch: GarrettBeatty/stacked/3
1263ff8 to
4e23b4b
Compare
Stacked PRs:
Description
This change adds progress listeners for "initiated", "complete", and "failed" lifecycle events for the MultiUploadCommand. Consumers can subscribe to these callbacks to receive notifications when a simple upload starts, finishes successfully, or fails. this is similar to #4059 but for multi part upload command.
Example usage
Motivation and Context
Testing
1 .84f23e9a-f014-44a6-b129-5d10e7aea8d6 - pass
2. Integration tests
Types of changes
Checklist
License