Skip to content

fix: Zip CSV uploads not working#1457

Merged
mofojed merged 4 commits intodeephaven:mainfrom
mattrunyon:jszip-fix
Aug 31, 2023
Merged

fix: Zip CSV uploads not working#1457
mofojed merged 4 commits intodeephaven:mainfrom
mattrunyon:jszip-fix

Conversation

@mattrunyon
Copy link
Copy Markdown
Collaborator

Fixes #1080. Fixes #1416

Updates jszip and uses the internal stream helper instead of nodestream which we would need a polyfill for. The progress on zip uploads is a bit odd since we use the unzip progress. It seems papaparse loads more from the stream while processing/before processing and can finish reading the zip before it's done processing chunks.

Also, uploading the tables seems to be a blocking operation. Not sure if that's an easy fix, but after the 50% mark on uploading a large zip, there are some blocks on the main thread (marching ants freezes as an indicator).

There is a ~2GB limit for JSZip it seems Stuk/jszip#777

@mattrunyon mattrunyon requested a review from mofojed August 23, 2023 20:28
@mattrunyon mattrunyon self-assigned this Aug 23, 2023
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 23, 2023

Codecov Report

Merging #1457 (fa16742) into main (938aa07) will decrease coverage by 0.02%.
The diff coverage is 7.14%.

@@            Coverage Diff             @@
##             main    #1457      +/-   ##
==========================================
- Coverage   45.70%   45.68%   -0.02%     
==========================================
  Files         513      514       +1     
  Lines       35045    35062      +17     
  Branches     8781     8779       -2     
==========================================
+ Hits        16017    16019       +2     
- Misses      18977    18992      +15     
  Partials       51       51              
Flag Coverage Δ
unit 45.68% <7.14%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
packages/console/src/csv/CsvInputBar.tsx 2.94% <0.00%> (ø)
packages/console/src/csv/CsvTypeParser.ts 44.29% <0.00%> (-1.23%) ⬇️
packages/console/src/csv/ZipStreamHelper.ts 0.00% <0.00%> (ø)
packages/console/src/csv/CsvParser.ts 35.06% <16.66%> (-0.08%) ⬇️

Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Gross. Good work!

@mofojed mofojed merged commit 08d0296 into deephaven:main Aug 31, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 31, 2023
@mattrunyon mattrunyon deleted the jszip-fix branch June 23, 2024 06:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to import zipped CSV file Update JSZip

2 participants