LargeFileUpload task #584

andrueastman · 2019-11-11T09:14:23Z

Changes proposed in this pull request

Builds upon fix for extra memory allocation when doing chunked upload( from PR Avoid extra memory allocation when doing chunked upload for large file #529 credit to @zsybupt )
Wraps the ChunkedUploadProvider in a LargeFileUpload task
Adds support for providing a callback mechanism for monitoring uploads

This work is done in accordance with the spec found at this link. https://github.com/microsoftgraph/msgraph-sdk-design/blob/master/tasks/FileUploadTask.md

Essentially, one may perform a large upload by following the steps below.

create an upload session

// Create upload session 
// POST /v1.0/drive/items/01KGPRHTV6Y2GOVW7725BZO354PWSELRRZ:/SWEBOKv3.pdf:/microsoft.graph.createUploadSession
var uploadSession = await graphClient.Drive.Items[itemId].ItemWithPath("SWEBOK.pdf").CreateUploadSession().Request().PostAsync();

Create the task using the session.

// Create task
var maxChunkSize = 320 * 1024; // 320 KB - Change this to your chunk size. 5MB is the default.
var largeFileUpload = new LargeFileUpload(uploadSession, graphClient, stream, maxChunkSize);

Create an upload monitor

    public class MyProgress : IProgressCallback
    {
        public void OnFailure(ClientException clientException)
        {
            Console.WriteLine(clientException.Message);
        }

        public void OnSuccess(DriveItem result)
        {
            Console.WriteLine("Download completed with id below");
            Console.WriteLine(result.Id);
        }

        public void UpdateProgress(long current, long max)
        {
            long percentage = (current * 100) / max ;
            Console.WriteLine("Upload in progress. " + current + " bytes of " + max + "bytes. " + percentage + " percent complete");
        }
    }

Upload the file

uploadedFile = await largeFileUpload.ResumeAsync(new MyProgress());

Alternatively, one may choose to do the file upload in slices by themselves by replacing step 3 and 4 with

// Setup the chunk request necessities
var slicesRequests = largeFileUpload.GetUploadSlicesRequests();
var trackedExceptions = new List<Exception>();
DriveItem itemResult = null;

//upload the chunks
foreach (var request in slicesRequests)
{
    // Send chunk request
    var result = await largeFileUpload.UploadSliceAsync(request, trackedExceptions);
    // Do your updates here: update progress bar, etc.
    Console.WriteLine($"File uploading in progress. {request.RangeEnd} of {stream.Length} bytes uploaded");

    if (result.UploadSucceeded)
    {
        itemResult = result.ItemResponse;
        Console.WriteLine($"File uploading complete");
    }
}

Notes

One can find a sample app built upon this PR(using this repo as a git submodule) to showcase and test the above usage at the following link.
https://github.com/andrueastman/UploadFileSample

Avoid extra memory allocation when doing chunked upload for large file

- Wraps the chunkedUploadProvider in another LargeFileUpload - Prevents breaking changes by keeping obsolete method - Performance tests done

MIchaelMainer

I want to review a bit more.

MIchaelMainer · 2019-11-13T00:24:58Z

src/Microsoft.Graph/Requests/Helpers/ChunkedUploadProvider.cs


                    if (result.UploadSucceeded)
                    {
+                        progressCallback?.OnSuccess(result.ItemResponse);// update the caller with sucess


MIchaelMainer · 2019-11-13T00:32:21Z

src/Microsoft.Graph/Requests/Helpers/ChunkedUploadProvider.cs

+        /// <param name="request">The UploadChunkRequest to make the request with.</param>
+        /// <param name="exceptionTrackingList">A list of exceptions to use to track progress. ChunkedUpload may retry.</param>
+        /// <returns></returns>
+        public virtual async Task<UploadChunkResult> GetChunkRequestResponseAsync(UploadChunkRequest request, ICollection<Exception> exceptionTrackingList)


I think this may be better called UploadChunkAsync().

MIchaelMainer · 2019-11-13T00:37:17Z

src/Microsoft.Graph/Tasks/IProgressCallBack.cs

+namespace Microsoft.Graph
+{
+    /// <summary>
+    /// Interface for to enbale handling of progress/errors for a long upload process. 


typo.

Additionally, we should make this generic enough to be used in other progress scenarios. This should be applicable to upload, download, or any other scenario where progress needs to be captured.

.NET has an interface built in https://docs.microsoft.com/en-us/dotnet/api/system.iprogress-1?view=netframework-4.8

MIchaelMainer · 2019-11-13T00:43:04Z

src/Microsoft.Graph/Tasks/IProgressCallBack.cs

+        /// Handler for scenario when process completes successfully
+        /// </summary>
+        /// <param name="result">The resulting file from the upload.</param>
+        void OnSuccess(DriveItem result);


Do we need to return a DriveItem? Could we make this some sort of result object? I'd like us to be reuse interface for other progress scenarios.

darrelmiller · 2019-11-13T20:37:35Z

99% of this code should be in Core. We need to make this generic so that it is not tied to DriveItem. We will want to use this code for large attachment uploads also.

darrelmiller · 2019-11-13T20:38:59Z

We cannot use the term Chunk anywhere in our interface or samples. This is not chunking.

peombwa · 2019-11-14T22:06:50Z

src/Microsoft.Graph/Requests/Helpers/ChunkedUploadProvider.cs

        /// <param name="readBuffer">The byte[] content to read from.</param>
        /// <param name="exceptionTrackingList">A list of exceptions to use to track progress. ChunkedUpload may retry.</param>
        /// <returns></returns>
+        [Obsolete("This overload is obsolete. Please use the overload that does not use the unnecessary a readBuffer parameter")]


It's worth informing our customers that the readBuffer value passed here will be ignored.

peombwa · 2019-11-14T22:10:28Z

src/Microsoft.Graph/Requests/Helpers/ChunkedUploadProvider.cs

+        /// <param name="progressCallback"><see cref="IProgressCallback"/> object to monitor the progress of the upload.</param>
        /// <returns>Item information returned by server.</returns>
-        public async Task<DriveItem> UploadAsync(int maxTries = 3, IEnumerable<Option> options = null)
+        public async Task<DriveItem> UploadAsync(int maxTries = 3, IEnumerable<Option> options = null , IProgressCallback progressCallback = null)


As @darrelmiller noted, lets consider using .Net's IProgress interface here.

peombwa · 2019-11-14T23:40:44Z

src/Microsoft.Graph/Requests/Helpers/ChunkedUploadProvider.cs


                await this.UpdateSessionStatusAsync().ConfigureAwait(false);
                uploadTries += 1;
                if (uploadTries < maxTries)


This retry logic will result in 3^3 retries (by default) for 429, 503 and 504 HTTP responses when the service doesn't return a successful result. The underlying HTTP client already has a RetryHandler that handles retries for 429, 503 and 504 responses.

Misiu · 2019-11-18T12:04:48Z

99% of this code should be in Core. We need to make this generic so that it is not tied to DriveItem. We will want to use this code for large attachment uploads also.

I'm waiting for a large file upload while sending emails. It was announced some time ago (https://docs.microsoft.com/en-us/graph/outlook-large-attachments?tabs=http), but its still unsupported here and in beta lib.

andrueastman · 2019-11-22T08:10:40Z

Thanks for all the feedback on this PR.
I am going to close this PR and limit changes to the service library to fixing the memory issue by creating a separate PR and move the generic UploadTask to Core in a short while so as to support both v1.0 and the beta libs.

8ggmaker and others added 4 commits August 20, 2019 23:11

avoid extra memory allocation when do chunked upload

fcdec71

Merge pull request #529 from zsybupt/dev

abc3ee8

Avoid extra memory allocation when doing chunked upload for large file

Adds support for progress object

aa56b54

- Wraps the chunkedUploadProvider in another LargeFileUpload - Prevents breaking changes by keeping obsolete method - Performance tests done

Merge branch 'dev' into andrueastman/FileUploadTask

c3c484d

andrueastman self-assigned this Nov 11, 2019

andrueastman marked this pull request as ready for review November 11, 2019 09:30

andrueastman requested review from MIchaelMainer, darrelmiller and peombwa November 11, 2019 09:30

MIchaelMainer reviewed Nov 13, 2019

View reviewed changes

peombwa reviewed Nov 14, 2019

View reviewed changes

andrueastman mentioned this pull request Nov 22, 2019

Chunked upload enhancements #592

Merged

andrueastman closed this Nov 22, 2019

andrueastman mentioned this pull request Nov 22, 2019

FileUpload Task microsoftgraph/msgraph-sdk-dotnet-core#65

Merged

andrueastman deleted the andrueastman/FileUploadTask branch December 3, 2019 05:41

LargeFileUpload task #584

LargeFileUpload task #584

Uh oh!

Conversation

andrueastman commented Nov 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this pull request

Notes

Uh oh!

MIchaelMainer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrelmiller commented Nov 13, 2019

Uh oh!

darrelmiller commented Nov 13, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Misiu commented Nov 18, 2019

Uh oh!

andrueastman commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

andrueastman commented Nov 11, 2019 •

edited

Loading