Skip to content

Conversation

@GarrettBeatty
Copy link
Contributor

@GarrettBeatty GarrettBeatty commented Dec 1, 2025

Description

  1. Add DownloadDirectory Initiated, Failed, and Completed events. These are similar to Add DownloadInitiated, Failed and Completed events #4079 but for directory events. These events are also different than the ObjectDownloadedFailedEvent from add failure policy to download directory #4151 (see explanation below)

ObjectDownloadFailedEvent:

  • Fires every time an individual file fails to download
  • Think of it as "this specific file couldn't be downloaded"
  • Can happen multiple times during one directory download (once per failed file)
  • Always includes details about which specific file failed and why

DownloadDirectoryFailedEvent:

  • Fires once when the entire directory download operation fails
  • Think of it as "the whole directory download operation is now stopping"
  • Only happens once per directory download attempt
  • Includes summary information about the overall operation (how many files succeeded/failed, total progress, etc.)

Key Scenarios:

  1. With "Continue on Failure" policy:

    • Individual files fail → ObjectDownloadFailedEvent fires for each one
    • Directory download continues and eventually completes
    • DownloadDirectoryFailedEvent does NOT fire (operation succeeded overall)
  2. With "Abort on Failure" policy:

    • First file fails → ObjectDownloadFailedEvent fires
    • Operation immediately aborts → DownloadDirectoryFailedEvent also fires
    • Both events fire in this case
  3. Pre-download failures (validation, listing S3 objects, etc.):

    • No individual files have been attempted yet
    • Only DownloadDirectoryFailedEvent fires
    • ObjectDownloadFailedEvent never fires because no specific file failed

The important distinction is: ObjectDownloadFailedEvent is about individual file failures, while DownloadDirectoryFailedEvent is about the overall operation failing.

Motivation and Context

To comply with the SEP.

Testing

  1. Integration Tests

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

Copy link
Contributor

Copilot AI left a 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 directory-level lifecycle events (Initiated, Completed, Failed) to the S3 TransferUtility's DownloadDirectory operation, providing developers with better visibility into the overall download directory process. It also includes minor adjustments to when lifecycle events are fired in related operations.

  • Addition of three new event types with progress information for download directory operations
  • Comprehensive integration tests validating the new event functionality
  • Event firing order adjustments in SimpleUploadCommand and DownloadCommand for consistency

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sdk/src/Services/S3/Custom/Transfer/TransferUtilityDownloadDirectoryRequest.cs Adds three new public events (Initiated, Completed, Failed) and corresponding event args classes with progress tracking
sdk/src/Services/S3/Custom/Transfer/Internal/DownloadDirectoryCommand.cs Implements event firing methods and integrates them into the download execution flow
sdk/src/Services/S3/Custom/Transfer/Internal/_bcl+netstandard/DownloadDirectoryCommand.cs Wraps execution in try-catch to fire events at appropriate lifecycle points (start, success, failure)
sdk/src/Services/S3/Custom/Transfer/Internal/_async/SimpleUploadCommand.async.cs Moves FireTransferInitiatedEvent before AsyncThrottler.WaitAsync for consistent event timing
sdk/src/Services/S3/Custom/Transfer/Internal/_async/DownloadCommand.async.cs Moves FireTransferInitiatedEvent before ValidateRequest for consistent event timing
sdk/test/Services/S3/IntegrationTests/TransferUtilityDownloadDirectoryLifecycleTests.cs Comprehensive integration test suite covering all three new events and their behavior
generator/.DevConfigs/9d07dc1e-d82d-4f94-8700-c7b57f872124.json Dev config marking the changes as a minor version bump with appropriate changelog
generator/.DevConfigs/9d07dc1e-d82d-4f94-8700-c7b57f872123.json Updates changelog message for consistency with new naming convention
sdk/global.json Adds SDK version specification (8.0.415)

@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/downloadprogress branch from c675340 to ec52c07 Compare December 1, 2025 16:20
@GarrettBeatty GarrettBeatty changed the title DownloadDirectoryProgress events DownloadDirectory Initiated, Failed and Completed Events Dec 1, 2025
{
ValidateRequest();

FireTransferInitiatedEvent();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this to the top just to be consistent

@GarrettBeatty GarrettBeatty force-pushed the gcbeatty/downloadprogress branch from ec52c07 to 6af787d Compare December 1, 2025 16:32
@GarrettBeatty GarrettBeatty requested a review from Copilot December 1, 2025 16:34
Copilot finished reviewing on behalf of GarrettBeatty December 1, 2025 16:36
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant