Support for download via byte stream#20
Open
elliotsayes wants to merge 11 commits intoardriveapp:mainfrom
Open
Support for download via byte stream#20elliotsayes wants to merge 11 commits intoardriveapp:mainfrom
elliotsayes wants to merge 11 commits intoardriveapp:mainfrom
Conversation
(fixed in fetch_api 1.0.1)
karlprieb
reviewed
Mar 21, 2023
karlprieb
reviewed
Mar 21, 2023
Contributor
|
Another thing I noticed is that the code is not retrying using |
karlprieb
reviewed
Mar 21, 2023
karlprieb
previously approved these changes
Mar 23, 2023
karlprieb
approved these changes
Mar 27, 2023
matibat
approved these changes
Apr 6, 2023
| environment: | ||
| sdk: '>=2.18.5 <3.0.0' | ||
| flutter: 3.3.9 | ||
| flutter: 3.7.0 |
There was a problem hiding this comment.
TODO for @matibat: check what are the downsides of bumping the version for localizations
Author
There was a problem hiding this comment.
Just FYR already fixed a bunch of localisation errors in the ArDrive-web PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A few problems I encountered that lead me to this particular solution:
Rangeheader, thearweave.netgateway will eventually rate limit your requests, even with large chunks. Gateway then does exponential backoff which can seriously impact download time.diostreams are a lie - it downloads a buffer and wraps it inStream.valuepackage:httpon web still use XHR requests, also without real stream support: Migrate to thefetchAPI dart-lang/http#595So to get around all of these issues:
fetchon web (usingfetch_clientlibrary instead ofardrive-http.tsto handle the more complex js/dart interop)HttpClienton mobile which just worksI've confirmed that the memory usage on web is stable. Not sure how to confirm on mobile
Caveats of this method:
fetch. Seems that this cannot be fixed: Support for flow control? Zekfad/fetch_client#3