-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add CSV max filesize check (#190) #191
base: master
Are you sure you want to change the base?
Add CSV max filesize check (#190) #191
Conversation
For the sake of simplicity I've added the file size check to I've also used |
@@ -17,7 +17,14 @@ | |||
|
|||
def can_preview(file): | |||
"""Determine if the given file can be previewed.""" | |||
return file.is_local() and file.has_extensions(".csv", ".dsv") | |||
max_file_size = current_app.config.get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, would it make sense to have a validator function (i.e. validate_csv
) to separate concerns?
This is done for other extensions (e.g. json)
Based on your comments, I've added a separate validation function (@alejandromumo ) and increased the max file size to @lnielsen I didn't know that a new csv previewer was added. How does it work with the max file size, will it simply not load the file if it's too big (as with XML/JSON I guess) or will it truncate the file (like with TXT)? I was assuming here it won't load the file since otherwise there might be parsing issues. If on the other hand it does truncate the file, the wording of |
❤️ Thank you for your contribution!
Description
Fixes #190 by means of adding a file size check to
invenio_previewer.extensions.csv_papaparsejs.can_preview
.Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Third-party code
If you've added third-party code (copy/pasted or new dependencies), please reach out to an architect.
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: