Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions generator/.DevConfigs/c49077d9-90b3-437f-b316-6d8d8833ae77.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"services": [
{
"serviceName": "S3",
"type": "minor",
"changeLogMessages": [
"Add FailurePolicy property to TransferUtilityUploadDirectoryRequest to allow configuration of failure handling behavior during directory uploads. The default behavior is set to abort on failure. Users can now choose to either abort the entire operation or continue uploading remaining files when a failure occurs.",
"Add ObjectUploadFailedEvent event to TransferUtilityUploadDirectoryRequest to notify users when an individual file upload fails during a directory upload operation. This event provides details about the failed upload, including the original request, the specific file request and the exception encountered."
]
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@
*
*/
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Text;
using Amazon.Runtime;
using System.Threading;
using Amazon.S3.Transfer.Model;

namespace Amazon.S3.Transfer.Internal
{
Expand All @@ -34,12 +36,15 @@ namespace Amazon.S3.Transfer.Internal
/// </summary>
internal partial class UploadDirectoryCommand : BaseCommand<TransferUtilityUploadDirectoryResponse>
{
private IFailurePolicy _failurePolicy;
private ConcurrentBag<Exception> _errors = new ConcurrentBag<Exception>();
TransferUtilityUploadDirectoryRequest _request;
TransferUtility _utility;
TransferUtilityConfig _config;

int _totalNumberOfFiles;
int _numberOfFilesUploaded;
int _numberOfFilesSuccessfullyUploaded;
long _totalBytes;
long _transferredBytes;

Expand All @@ -48,6 +53,10 @@ internal UploadDirectoryCommand(TransferUtility utility, TransferUtilityConfig c
this._utility = utility;
this._request = request;
this._config = config;
_failurePolicy =
request.FailurePolicy == FailurePolicy.AbortOnFailure
? new AbortOnFailurePolicy()
: new ContinueOnFailurePolicy(_errors);
}

internal TransferUtilityUploadRequest ConstructRequest(string basePath, string filepath, string prefix)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@
* permissions and limitations under the License.
*/

using Amazon.S3.Model;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

Expand All @@ -30,24 +24,5 @@ internal abstract partial class BaseCommand<TResponse> where TResponse : class
/// Executes the command and returns a typed response
/// </summary>
public abstract Task<TResponse> ExecuteAsync(CancellationToken cancellationToken);

protected static async Task ExecuteCommandAsync<T>(BaseCommand<T> command, CancellationTokenSource internalCts) where T : class
{
try
{
await command.ExecuteAsync(internalCts.Token)
.ConfigureAwait(continueOnCapturedContext: false);
}
catch (Exception exception)
{
if (!(exception is OperationCanceledException))
{
// Cancel scheduling any more tasks.
// Cancel other upload requests.
internalCts.Cancel();
}
throw;
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Amazon.S3.Transfer.Model;

namespace Amazon.S3.Transfer.Internal
{
Expand Down Expand Up @@ -87,10 +88,29 @@ public override async Task<TransferUtilityUploadDirectoryResponse> ExecuteAsync(
// responses and throw the original exception.
break;
}

var uploadRequest = ConstructRequest(basePath, filepath, prefix);
var uploadCommand = _utility.GetUploadCommand(uploadRequest, sharedHttpRequestThrottler);

var task = ExecuteCommandAsync(uploadCommand, internalCts);

Copy link
Contributor

Choose a reason for hiding this comment

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

can we add debug logging to uploaddirectorycommand and download directory command related to concurrency and everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Action<Exception> onFailure = (ex) =>
{
this._request.OnRaiseObjectUploadFailedEvent(
new ObjectUploadFailedEventArgs(
this._request,
uploadRequest,
ex));
};

var task = _failurePolicy.ExecuteAsync(
async () => {
var command = _utility.GetUploadCommand(uploadRequest, sharedHttpRequestThrottler);
await command.ExecuteAsync(internalCts.Token)
.ConfigureAwait(false);
Interlocked.Increment(ref _numberOfFilesSuccessfullyUploaded);
},
onFailure,
internalCts
);

pendingTasks.Add(task);
}
finally
Expand All @@ -108,7 +128,17 @@ await TaskHelpers.WhenAllOrFirstExceptionAsync(pendingTasks, cancellationToken)
sharedHttpRequestThrottler?.Dispose();
}

return new TransferUtilityUploadDirectoryResponse();
return new TransferUtilityUploadDirectoryResponse
{
ObjectsUploaded = _numberOfFilesSuccessfullyUploaded,
ObjectsFailed = _errors.Count,
Errors = _errors.ToList(),
Result = _errors.Count == 0 ?
DirectoryResult.Success :
(_numberOfFilesSuccessfullyUploaded > 0 ?
DirectoryResult.PartialSuccess :
DirectoryResult.Failure)
};
}

private Task<string[]> GetFiles(string path, string searchPattern, SearchOption searchOption, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
using Amazon.Util;
using System.Globalization;
using Amazon.Runtime.Internal;
using Amazon.S3.Transfer.Model;

namespace Amazon.S3.Transfer
{
Expand All @@ -42,6 +43,44 @@ public class TransferUtilityUploadDirectoryRequest : BaseUploadRequest
string _keyPrefix;
private bool _uploadFilesConcurrently = false;
SearchOption _searchOption = SearchOption.TopDirectoryOnly;
private FailurePolicy failurePolicy = FailurePolicy.AbortOnFailure;

/// <summary>
/// Gets or sets the failure policy for the upload directory operation.
/// Determines whether the operation should abort or continue when a failure occurs during upload.
/// The default value is <see cref="FailurePolicy.AbortOnFailure"/>.
/// </summary>
public FailurePolicy FailurePolicy
{
get { return this.failurePolicy; }
set { this.failurePolicy = value; }
}

/// <summary>
/// Occurs when an individual object fails to upload during an UploadDirectory operation.
/// </summary>
/// <remarks>
/// Subscribers will receive a <see cref="ObjectUploadFailedEventArgs"/> instance containing
/// the original <see cref="TransferUtilityUploadDirectoryRequest"/>, the failed
/// <see cref="TransferUtilityUploadRequest"/>, and the exception that caused the failure.
/// This event is raised on a background thread by the transfer utility.
/// </remarks>
/// <example>
/// request.ObjectUploadFailedEvent += (sender, args) =>
/// {
/// // inspect args.DirectoryRequest, args.ObjectRequest, args.Exception
/// };
/// </example>
public event EventHandler<ObjectUploadFailedEventArgs> ObjectUploadFailedEvent;

/// <summary>
/// Internal helper used by the transfer implementation to raise the <see cref="ObjectUploadFailedEvent"/>.
/// </summary>
/// <param name="args">The details of the failed object upload.</param>
internal void OnRaiseObjectUploadFailedEvent(ObjectUploadFailedEventArgs args)
{
ObjectUploadFailedEvent?.Invoke(this, args);
}

/// <summary>
/// Gets or sets the directory where files are uploaded from.
Expand Down Expand Up @@ -382,4 +421,73 @@ public UploadDirectoryFileRequestArgs(TransferUtilityUploadRequest request)
/// </summary>
public TransferUtilityUploadRequest UploadRequest { get; set; }
}

/// <summary>
/// Provides data for <see cref="TransferUtilityUploadDirectoryRequest.ObjectUploadFailedEvent"/>
/// which is raised when an individual object fails to upload during an
/// UploadDirectory operation.
/// </summary>
/// <remarks>
/// Instances of this class are created by the transfer implementation and
/// passed to event subscribers. The instance contains the original directory
/// upload request (<see cref="TransferUtilityUploadDirectoryRequest"/>),
/// the per-object upload request that failed (<see cref="TransferUtilityUploadRequest"/>),
/// and the exception that caused the failure.
/// </remarks>
/// <example>
/// <code>
/// var request = new TransferUtilityUploadDirectoryRequest { /* ... */ };
/// request.ObjectUploadFailedEvent += (sender, args) =>
/// {
/// // args.DirectoryRequest: original directory request
/// // args.ObjectRequest: upload request for the failed object
/// // args.Exception: exception thrown during the object upload
/// Console.WriteLine($"Failed to upload {args.ObjectRequest.Key}: {args.Exception}");
/// };
/// </code>
/// </example>
public class ObjectUploadFailedEventArgs : EventArgs
{
/// <summary>
/// Initializes a new instance of the <see cref="ObjectUploadFailedEventArgs"/> class.
/// </summary>
/// <param name="directoryRequest">The original <see cref="TransferUtilityUploadDirectoryRequest"/> that initiated the directory upload.</param>
/// <param name="objectRequest">The <see cref="TransferUtilityUploadRequest"/> representing the individual object upload that failed.</param>
/// <param name="exception">The <see cref="Exception"/> that caused the object upload to fail.</param>
internal ObjectUploadFailedEventArgs(
TransferUtilityUploadDirectoryRequest directoryRequest,
TransferUtilityUploadRequest objectRequest,
Exception exception)
{
DirectoryRequest = directoryRequest;
ObjectRequest = objectRequest;
Exception = exception;
}

/// <summary>
/// Gets the original <see cref="TransferUtilityUploadDirectoryRequest"/> that initiated the directory upload.
/// </summary>
/// <value>
/// The directory-level request that configured the overall UploadDirectory operation.
/// </value>
public TransferUtilityUploadDirectoryRequest DirectoryRequest { get; private set; }

/// <summary>
/// Gets the <see cref="TransferUtilityUploadRequest"/> for the individual object that failed to upload.
/// </summary>
/// <value>
/// Contains per-object parameters such as the S3 key and version id (if set).
/// </value>
public TransferUtilityUploadRequest ObjectRequest { get; private set; }

/// <summary>
/// Gets the <see cref="Exception"/> that caused the object upload to fail.
/// </summary>
/// <value>
/// The exception thrown by the underlying upload operation. Can be an <see cref="Amazon.S3.AmazonS3Exception"/>,
/// <see cref="Amazon.Runtime.AmazonClientException"/>, <see cref="IOException"/>, or other exception type depending
/// on the failure mode.
/// </value>
public Exception Exception { get; private set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
*
*/

using Amazon.Runtime;
using System;
using System.Collections.Generic;
using Amazon.S3.Transfer.Model;

namespace Amazon.S3.Transfer
{
Expand All @@ -30,6 +32,24 @@ namespace Amazon.S3.Transfer
/// </summary>
public class TransferUtilityUploadDirectoryResponse
{
// Empty placeholder class - properties will be added in future iterations
/// <summary>
/// The number of objects that have been successfully uploaded.
/// </summary>
public long ObjectsUploaded { get; set; }

/// <summary>
/// The number of objects that failed to upload. Zero if all succeeded.
/// </summary>
public long ObjectsFailed { get; set; }

/// <summary>
/// The collection of exceptions encountered when uploading individual objects.
/// </summary>
public IList<Exception> Errors { get; set; }

/// <summary>
/// Overall result of the directory upload operation.
/// </summary>
public DirectoryResult Result { get; set; }
}
}
1 change: 1 addition & 0 deletions sdk/src/Services/S3/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

[assembly: InternalsVisibleTo("AWSSDK.UnitTests.S3.NetFramework, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db5f59f098d27276c7833875a6263a3cc74ab17ba9a9df0b52aedbe7252745db7274d5271fd79c1f08f668ecfa8eaab5626fa76adc811d3c8fc55859b0d09d3bc0a84eecd0ba891f2b8a2fc55141cdcc37c2053d53491e650a479967c3622762977900eddbf1252ed08a2413f00a28f3a0752a81203f03ccb7f684db373518b4")]
[assembly: InternalsVisibleTo("AWSSDK.UnitTests.NetFramework, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db5f59f098d27276c7833875a6263a3cc74ab17ba9a9df0b52aedbe7252745db7274d5271fd79c1f08f668ecfa8eaab5626fa76adc811d3c8fc55859b0d09d3bc0a84eecd0ba891f2b8a2fc55141cdcc37c2053d53491e650a479967c3622762977900eddbf1252ed08a2413f00a28f3a0752a81203f03ccb7f684db373518b4")]
[assembly: InternalsVisibleTo("AWSSDK.IntegrationTests.S3.NetFramework, PublicKey=0024000004800000940000000602000000240000525341310004000001000100db5f59f098d27276c7833875a6263a3cc74ab17ba9a9df0b52aedbe7252745db7274d5271fd79c1f08f668ecfa8eaab5626fa76adc811d3c8fc55859b0d09d3bc0a84eecd0ba891f2b8a2fc55141cdcc37c2053d53491e650a479967c3622762977900eddbf1252ed08a2413f00a28f3a0752a81203f03ccb7f684db373518b4")]
Copy link
Contributor

Choose a reason for hiding this comment

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

this file and the csproj files get auto generated. there is a AssemblyInfo.tt file that generates these. so you need to modify that and then rerun the generator as well to recreate the csproj files

Copy link
Contributor

Choose a reason for hiding this comment

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

Making something internal visible to an integration tests seems like an anti-pattern... Is it really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get where you're coming from but in our current implementation we don't return a response to the end user so i cant use the exact same public interface. We plan on adding one that returns the response in the future, at which point we can remove this anti-pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated AssemblyInfo.tt

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry if I'm not following, but are we trying to reference from the test project then? (I thought it was this interface but from your comment it doesn't exist yet?)

Copy link
Contributor

Choose a reason for hiding this comment

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

ive made the new api here #4187 probably we can remove this after we get it all merged in

[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]
[assembly: AssemblyConfiguration("")]
[assembly: AssemblyProduct("Amazon Web Services SDK for .NET")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,23 @@
<GenerateAssemblyFileVersionAttribute>false</GenerateAssemblyFileVersionAttribute>
<GenerateAssemblyDescriptionAttribute>false</GenerateAssemblyDescriptionAttribute>
<GenerateBindingRedirectsOutputType>true</GenerateBindingRedirectsOutputType>
<SignAssembly>true</SignAssembly>
<NoWarn>CS1591,CS0612,CS0618</NoWarn>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<Choose>
<When Condition=" '$(AWSKeyFile)' == '' ">
<PropertyGroup>
<AssemblyOriginatorKeyFile>../../awssdk.dll.snk</AssemblyOriginatorKeyFile>
</PropertyGroup>
</When>
<Otherwise>
<PropertyGroup>
<AssemblyOriginatorKeyFile>$(AWSKeyFile)</AssemblyOriginatorKeyFile>
</PropertyGroup>
</Otherwise>
</Choose>

<ItemGroup>
<Compile Remove="**/**" />
Expand Down
Loading
Loading