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

feat(upload): add progressTotal to event payload #2033

Merged
merged 5 commits into from
Nov 11, 2024

Conversation

514sid
Copy link
Contributor

@514sid 514sid commented Nov 11, 2024

This pull request addresses an issue with progress tracking during downloads. Previously, progress was updated using chunk.len(), which only reflected the size of each individual chunk rather than the cumulative total.

Changes:

Progress Calculation: Replaced chunk.len() with stats.total_transferred for progress, ensuring it accurately reflects the cumulative download progress from start to end.

TransferStats Update: Added total_transferred to TransferStats to maintain an ongoing count of all transferred bytes without resetting per granularity interval.

@514sid 514sid requested a review from a team as a code owner November 11, 2024 15:48
@FabianLars
Copy link
Member

This is not a bug and has always been the expected behavior. (Expected from us at least, there were quite a few users confused by this)

Since we're now keeping track of much more data than before the transfer_speed update i wouldn't mind adding a new field for this (keeping progress to represent the chunk size).

In v3 we could consider renaming the progress field but now this would break too many apps.

Copy link
Contributor

github-actions bot commented Nov 11, 2024

Package Changes Through 08013d4

There are 7 changes which include upload with minor, upload-js with minor, deep-link with patch, deep-link-js with patch, fs with patch, fs-js with patch, localhost with minor

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
api-example 2.0.5 2.0.6
api-example-js 2.0.2 2.0.3
deep-link-example-js 2.0.0 2.0.1
deep-link 2.0.1 2.0.2
deep-link-js 2.0.0 2.0.1
fs 2.0.3 2.0.4
fs-js 2.0.2 2.0.3
dialog 2.0.3 2.0.4
http 2.0.3 2.0.4
localhost 2.0.1 2.1.0
persisted-scope 2.0.3 2.0.4
single-instance 2.0.1 2.0.2
upload 2.1.0 2.2.0
upload-js 2.1.0 2.2.0

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

@514sid
Copy link
Contributor Author

514sid commented Nov 11, 2024

This is not a bug and has always been the expected behavior. (Expected from us at least, there were quite a few users confused by this)

Since we're now keeping track of much more data than before the transfer_speed update i wouldn't mind adding a new field for this (keeping progress to represent the chunk size).

In v3 we could consider renaming the progress field but now this would break too many apps.

How about adding a new field, for example, total_progress or total_transferred, to track cumulative progress while keeping progress for the chunk size?

@FabianLars
Copy link
Member

Well, i just said the same so yes, i agree :D

i'd go for total_progress or progress_total and leave transfer* for just the speed stuff.

@FabianLars
Copy link
Member

Thanks, could you also add a changefile like this one? https://github.com/VirtualPirate/plugins-workspace/blob/aa7b4c39ce1d3893f6fb72a06046f738f956fe52/.changes/add-transfer-speed.md

@514sid
Copy link
Contributor Author

514sid commented Nov 11, 2024

progress_total

It's probably more convenient that way

Copy link
Member

@FabianLars FabianLars left a comment

Choose a reason for hiding this comment

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

thank you

@514sid
Copy link
Contributor Author

514sid commented Nov 11, 2024

thank you

thank you too
I really appreciate all the work you’re doing — it’s awesome

@FabianLars FabianLars changed the title Fix download progress reporting feat(upload): add progressTotal to event payload Nov 11, 2024
@FabianLars FabianLars merged commit 5dadd20 into tauri-apps:v2 Nov 11, 2024
18 checks passed
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.

2 participants