Skip to content
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

YMPT™: Yet More Progress Tweaks #1376

Merged
merged 4 commits into from Apr 4, 2019

Conversation

Projects
None yet
2 participants
@goto-bus-stop
Copy link
Member

commented Mar 25, 2019

#1093 accidentally omitted file size reporting for GDrive/Dropbox uploads, which threw off client-side progress reporting. The first commit adds it back.

Unsized files (like instagram photos) now are stored with size: null instead of 0. Progress calculations used bytesTotal == null to check for unsized files, so this aligns the two properties. It also makes sure that unsized and 0-byte files are treated differently. Unsized files mean "this may be any size, we don't know" while empty files are just empty and we know it.

We've had several PRs to this stuff now so it needs more tests to avoid breakage in the future. Opening as a draft PR now so the … progress … is visible.

goto-bus-stop added some commits Mar 25, 2019

@@ -27,14 +27,14 @@ Object {
"name": "foo.jpg",
"preview": undefined,
"progress": Object {
"bytesTotal": 0,
"bytesTotal": null,
"bytesUploaded": 0,
"percentage": 0,

This comment has been minimized.

Copy link
@arturi

arturi Mar 26, 2019

Collaborator

Should all those be null then? Otherwise bytesTotal is null, but bytesUploaded is 0?

This comment has been minimized.

Copy link
@goto-bus-stop

goto-bus-stop Apr 1, 2019

Author Member

I only changed bytesTotal because bytesUploaded: 0 is actually accurate, we did upload precisely 0 bytes, while bytesTotal: 0 was a lie (we don't know if the file is 0 bytes)

@arturi

This comment has been minimized.

Copy link
Collaborator

commented Mar 26, 2019

Thanks for attacking this! Crazy we didn’t have file sizes by accident for a while.

goto-bus-stop added some commits Apr 1, 2019

@goto-bus-stop goto-bus-stop marked this pull request as ready for review Apr 1, 2019

@arturi

This comment has been minimized.

Copy link
Collaborator

commented Apr 4, 2019

I’ve gotten this:

Screen Shot 2019-04-04 at 13 05 15

and other weird 1260% a couple of times, but can’t reproduce consistently. Merging and will see on master.

@arturi arturi merged commit 6a5c25f into master Apr 4, 2019

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@arturi arturi deleted the fix/progress-again branch Apr 4, 2019

@arturi arturi referenced this pull request Apr 25, 2019

Merged

Non parallel uploads #1497

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.