Fix 1805 #2087
Fix 1805 #2087
Conversation
| for i, (fid, path, data_type) in enumerate(a.filepaths): | ||
| # validate access only of the first artifact filepath, | ||
| # the rest have the same permissions | ||
| if (i == 0 and not vfabu(user, fid)): |
There was a problem hiding this comment.
Can you add a tests for this case?
qiita_pet/handlers/download.py
Outdated
| if (i == 0 and not vfabu(user, fid)): | ||
| to_add = False | ||
| break | ||
| if data_type == 'directory': |
qiita_pet/handlers/download.py
Outdated
| spath = path[basedir_len:] | ||
| to_download.append((path, spath, spath)) | ||
| else: | ||
| to_download.append((path, path, path)) |
There was a problem hiding this comment.
Not sure how to add a test, added a comment explaining why
| % a.id)) | ||
|
|
||
| # If we don't have nginx, write a file that indicates this | ||
| all_files = '\n'.join(["- %s /protected/%s %s" % (getsize(fp), sfp, n) |
There was a problem hiding this comment.
I liked having the message "This installation of Qiita was not equipped with nginx, ... ", is it possible to add it?
There was a problem hiding this comment.
According the the mod_zip documentation you cannot have any extra lines, so I don't think it is possible to add that information.
|
👍 |
qiita_pet/handlers/download.py
Outdated
|
|
||
| # If we don't have nginx, write a file that indicates this | ||
| all_files = '\n'.join(["%s %s /protected/%s %s" | ||
| % (compute_checksum(fp), getsize(fp), sfp, n) |
There was a problem hiding this comment.
compute_checksum is going to be expensive as it requires reading each file in whole from the filesystem. This is multiplied by the potential number of files collected from the deeply nested if statements. Are you sure this should be executed by tornado?
There was a problem hiding this comment.
Agree with @wasade. Note that this was a test and we are removing.
|
Is there anything else needed in this PR? |
|
Just to keep everyone in the loop: We realized that in some occasions the zip downloaded will not open in Macs because due to the size of the files it requires unzip >= 6.00. Thus, we decided to host a Mac version of unzip 6.00 in the FTP (cause the only way to install it is via brew), and add a GUI line under the button pointing to new documentation explaining the issue and fix. |
|
Closing as I'm gonna create another PR with the requested fixes as @josenavas requested it. |
|
Oh, I think I have seen this before, what about using gzip? My past experience was that large zip files could not be opened unless I used |
|
mod_gzip for nginx doesn't really work like mod_zip. In other words, we can't use mod_gzip for what we are doing here ... |
This grab's @antgonza's code and fixes it so the download through nginx works correctly.
@antgonza @ElDeveloper are you able to do a review?