Re-implement download session to restore bounded memory usage #271
Conversation
This file contains 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
added 15 commits
January 23, 2018 19:51
- zip-stream is core library that implements creating a zip file in memory - conduit-combinators provides some helper functions to connect zip-stream with our web application - blaze-builder provides some helper functions for connecting as well
- duplicate, simplify logic from wai-conduit to use responseStream send with conduit source - create an instance of ResponseData for conduit source
… meta data - zipResponse2 will now build a flat set of entries (instead of nested) to match parameter types of zip-stream library - disable estimating zip size for now
- build a stream and allow wai to serve it - generate zips with no compression (directory packing only), and allow large files - accept list of entry/data pairs to properly match zip-stream interface
- as before create a directory entry corresponding to the container (session) - directory entry currently does not include link comment as before - contents of zip directory entry is empty, as required by zip format - introduce utility for creating zipentry
…till work - volume zip and container zip shared a function, use the newer function only in container zip
- for now, generating assets with a name, size, and content, no comment yet - note: empty directory with no assets was causing zip failure
- try sending only asset entries, no directory entry
- bad content-length of 0 was causing error
- new library requires full path to generate proper folders in zip
- timestamp uses current time converted to utc
- can't add comments like before because zip-stream doesn't support them
|
If regression tests pass, remove dead code and merge. |
ghost
closed this
- size computed was probably wrong, regardless only skipping the content-length header works, only works on linux chrome
|
Working partially, hit dead-end for now. Switching to alternative library. |
ghost
reopened this
ghost
closed this
ghost
deleted the
ghost
mentioned this pull request
This pull request was closed.
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.
Most likely upgrading from haskell compiler 7.10.3 to 8 introduced a change in behavior that broke an optimization that our homegrown zip generation logic relied upon
Use third party libraries to generate zip files in memory for download session, restoring the same or better memory behavior as old version of code.
zip-streamappears to be one of the only libraries that offer this functionality. (There is also an in progress implementation of the feature forziphere - mrkkrp/zip#20)https://github.com/databrary/databrary/pull/269/files