Skip to content
This repository has been archived by the owner on May 12, 2023. It is now read-only.

Add upload and download progress callbacks for file transfer #63

Closed
wants to merge 3 commits into from

Conversation

frmatthew
Copy link

What kind of change does this PR introduce?

This PR adds support for upload and download callbacks for file transfer operations. Fixes supabase/supabase-flutter#308

What is the current behavior?

At present there is no support for download/upload callbacks. This is an oft requested feature for the storage API, and one that is needed for various production uses of the Dart Supabase packages.

What is the new behavior?

Now, the four upload operations in the StorageFileApi (upload, uploadBinary, update, and updateBinary) include the optional onSendProgress parameter, which is called for regular byte chunks that are transmitted. Likewise, the download function includes an optional onReceiveProgress callback, which functions in the same way.

These callbacks have the signature void Function(int count, int total), where count is the number of bytes transferred up to that moment, and total is the total expected number of bytes for the whole transfer operation (or -1 if unknown). The total bytes includes not only the file size, but also the request/response envelope size. This signature is used as it is allows precise tracking of not only the relative progress (count.toDouble() / total), but also the actual bytes in question, which can be useful in reporting and logging transferred data.

Additional context

For reference, some of the related discussions on this topics include:

Integration of the TUS protocol (see supabase/supabase-flutter#438) will likely affect the upload progress implementation here. In that case, this PR could serve as a temporary stopgap until TUS support is added, and also it could serve as an opportunity to establish the desired API interface (parameter names, etc.) for progress callbacks.

@frmatthew
Copy link
Author

Some possible considerations/discussion points remaining for this PR:

  • Is 4k a good chunk size to use in reporting upload progress (see _ChunkedMultipartRequest in fetch.dart)?
  • Are the progress callback parameter names and API styles acceptable?
  • Will this allow transparent integration of TUS uploads?
  • Probably need to add some unit tests for the progress callbacks?

@dshukertjr
Copy link
Member

This looks very nice, but we want to be able to align the APIs with other client libraries when upload progress lands, so at this point we will not merge this.

Also, when I uploaded a video of around 3.8 MB, It took about 300 - 400 milliseconds for the progress to get to 100% (when count == total), but the upload methods took about 1,400 milliseconds to complete.

final start = DateTime.now();
await storageClient.from('bucket').upload(
  '/my/path,
  file,
  fileOptions: const FileOptions(upsert: true),
  onSendProgress: (count, total) {
    final progress = count / total;
    final now = DateTime.now();
    if(progress == 1) print('$percent, ${now.difference(start).inMilliseconds} milliseconds'); // 300 - 400 milliseconds 
  },
);
final end = DateTime.now();
print('done: ${end.difference(start).inMilliseconds} milliseconds'); // 1,400 milliseconds

@dshukertjr dshukertjr closed this Apr 17, 2023
@frmatthew
Copy link
Author

Oh, nice catch. Some tracing measurements through _handleMultipartRequest with a larger file shows:

flutter: MultipartFile.fromBytes and file.readAsBytesSync(): 13ms
flutter: Create _ChunkedMultipartRequest and await r.retry<http.StreamedResponse>: 13380
flutter: _handleResponse: 2ms
flutter: Reported upload time (in final onSendRequest callback): 12100ms
flutter: Total measured upload time: 13395ms

Still around 1s of difference for me too, maybe because the upload callback is reporting buffered data, not actual uploaded data? Oh well...

@frmatthew frmatthew deleted the add-download-progress branch April 17, 2023 06:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to listen or react to storage upload status
2 participants