feat: Backend implementation for bulk attachment downloading #555
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
liangyuanruo
reviewed
Nov 2, 2020
There was a problem hiding this comment.
I suspect the Transform will cause issues because of a lack of backpressure management.
We might be able to resolve this by using pipeline, but let me test the implementation with a form with sizeable submission.
|
I am not sure if it will create significant backpressure because signing URLs doesn't require additional network calls when you have a valid IAM key. We can test this out in staging though. |
liangyuanruo
force-pushed
the
frank-refactor-download
branch
from
b1f6365 to
9772217
Compare
…g CSV decryption webworkers
|
@liangyuanruo, just tested. For signing and downloading 10,000 submissions, the heap usage went from 96 MB up to ~115 MB according to the profiler. The entire download took 15 seconds. I think this means that there are no significant memory pressure issues. |
frankchn
force-pushed
the
frank-refactor-download
branch
from
9772217 to
90c5507
Compare
frankchn
force-pushed
the
frank-refactor-download
branch
from
90c5507 to
5ce593f
Compare
liangyuanruo
approved these changes
Nov 11, 2020
karrui
added a commit
that referenced
this pull request
Nov 16, 2020
this is a refactor of the function recently added in #555
karrui
added a commit
that referenced
this pull request
Nov 17, 2020
* feat(EncryptSubModel): add getSubmissionCursorByFormId static method * feat(EncryptSubSvc): add getSubmissionCursor service fn * feat(EncryptSubCtl): add handler for streaming encrypted response api * ref(AdminFormsRoutes): use migrated handler for streaming responses * feat(AdminFormsRoutes): add celebrate validtn for stream query params * build: update tsconfig.build to allow inconsistent casing due to transpiling code, some packages with lowercased type files will encounter some errors like: TS1261: Already included file name 'path/to/node_modules/@types/JSONStream/index.d.ts' differs from file name 'path/to/node_modules/@types/jsonstream/index.d.ts' only in casing. * test(EncryptSubSvc): add tests for getSubmissionCursor * fix: use local copy of jsonstream types * test(EncryptSubModel): add tests for getSubmissionCursorByFormId * ref: use StatusCodes enums instead of hardcoded numbers * feat: remove `end` event handler in stream the close handler should be enough to handle everything. From testing, the end event is not called anyways * ref(AdminFormsRoutes): extract out YYYYMMDD regex into variable * fix(AdminFormsRoutes): use Joi.object.and instead of object.with `with` allows for a query with only `endDate` since only `startDate` is required, which is wrong * ref(EncryptSubSvc): add transformAttachmentMetaStream function this is a refactor of the function recently added in #555 * feat(EncryptSubCtl): insert transform stream into download pipeline * feat: remove unused stream responses handler * feat(EncryptSubSvc): add logging to error also use for of loop for correct callback return * feat(EncryptSubCtl): add error handler for attachment pipe errors * test(EncryptSubSvc): add tests for transformAttachmentMetaStream * chore: trigger travis rebuild
Merged
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.
Backend implementation for bulk attachment downloading using existing CSV decryption workers.