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

File Download: Multiple file download size limit restricts zip download to 10MB. Needs to be configurable again, have sensible default. #2686

Closed
kcondon opened this issue Oct 23, 2015 · 8 comments

Comments

@kcondon
Copy link
Contributor

kcondon commented Oct 23, 2015

Multiple file download size limit restricts zip download to 10MB. Needs to be configurable again, have sensible default.

@pdurbin
Copy link
Member

pdurbin commented Jan 15, 2016

https://help.hmdc.harvard.edu/Ticket/Display.html?id=229652 is an example of where this bug has been noted.

In addition, I'm working with @pameyer on stuff for @sbgrid and I noticed a MANIFEST.TXT file in dataverse_files.zip with contents that start with lines like this:

.adxv_beam_center (application/octet-stream) 19 bytes.
files.sha (application/octet-stream) 2742 bytes.
kdc_apr11_d3_1_001.img (application/octet-stream) 18875392 bytes.
kdc_apr11_d3_1_002.img (application/octet-stream) skipped because the total size of the download bundle exceeded the limit of 10485760 bytes.
kdc_apr11_d3_1_003.img (application/octet-stream) skipped because the total size of the download bundle exceeded the limit of 10485760 bytes.
kdc_apr11_d3_1_004.img (application/octet-stream) skipped because the total size of the download bundle exceeded the limit of 10485760 bytes.

This is to say that dataverse_files.zip didn't have all the files in it, despite my attempt to download all files as zip.

The value of "10485760 bytes" seems to come from src/main/java/edu/harvard/iq/dataverse/dataaccess/DataFileZipper.java because when I change (on my laptop) DEFAULT_ZIPFILE_LIMIT from 10 * 1024 * 1024 to Long.MAX_VALUE I'm able to download all the files as zip. That is to say, after I made this local change the dataverse_files.zip file I download is the full is 293 MB I expect.

Basically, I'm simply confirming what @kcondon has already reported. From what I can tell the 10 MB (10485760 bytes) restriction is indeed hard coded when downloading files as zip. I agree that this should definitely be configurable. And maybe it should be set to unlimited by default. When downloading multiple files as zip I can imagine that "all files" would be a popular desire.

@pdurbin
Copy link
Member

pdurbin commented Jan 20, 2016

@landreev as discussed at this morning's standup I'm changing the assignee of this issue to you and making the milestone 4.3 for now. If you could fix the "ZipDonwloadLimit" typo while you're in there I'd appreciate it.

@pdurbin pdurbin modified the milestones: 4.3, Not Assigned to a Release Jan 20, 2016
@scolapasta scolapasta modified the milestones: 4.3, 4.2.4 Feb 4, 2016
@landreev
Copy link
Contributor

landreev commented Feb 5, 2016

The fix is checked in, in the 2686-2907-bugfixpatch-424 branch.
For the record, it wasn't just the typo in the setting name; misspelled or not, that setting was never consulted by the download instance generating the output.

@landreev landreev assigned kcondon and unassigned landreev Feb 5, 2016
@landreev
Copy link
Contributor

landreev commented Feb 5, 2016

The branch: 2686-2907-bugfixpatch-424
The size limit option for the zipped downloads: ZipDownloadLimit
Should be set to the number of bytes.
For example:
curl -X PUT -d 100000000 "http://localhost:8080/api/admin/settings/:ZipDownloadLimit"
to set it to 100MB.

@kcondon
Copy link
Contributor Author

kcondon commented Feb 8, 2016

Note: the default is 10MB but it can be changed by the switch mentioned. This should be documented.

@landreev
Copy link
Contributor

landreev commented Feb 8, 2016

OK, the default is now 100MB (like what we had in DVN 3.?)
Also, changed the logic of size checks - it's now checking the size of the file *before
adding it to the zip stream, not after. So, if TOTAL + CURRENT FILE SIZE > LIMIT, then CURRENT FILE doesn't get added. Back in DVN and up until now we were checking this after the file was added to the stream.

Yes, if you have the limit set to N, and none of the files the user selected are smaller than N, that means the resulting zip will have no files, and the Manifest explaining why the files were skipped.

@kcondon
Copy link
Contributor Author

kcondon commented Feb 8, 2016

Works as described. Closing.

@pdurbin
Copy link
Member

pdurbin commented Feb 10, 2016

Note: the default is 10MB but it can be changed by the switch mentioned. This should be documented.

I'm a little surprised this got past our new Request for Integration (RFI) process but I suppose it hasn't been finalized. Anyway, I added it to a list I just started at #2944.

@kcondon kcondon removed this from the 4.2.4 milestone Feb 10, 2016
@kcondon kcondon removed their assignment Feb 10, 2016
@kcondon kcondon self-assigned this Feb 10, 2016
@kcondon kcondon added this to the 4.2.4 milestone Feb 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants