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

Refuse uploading too big .zip files #1049

Merged
merged 1 commit into from
Oct 25, 2017

Conversation

bruntib
Copy link
Contributor

@bruntib bruntib commented Oct 17, 2017

Fixes #840

@@ -315,6 +316,11 @@ def main(args):

try:
assemble_zip(args.input, zip_file, client)

if os.stat(zip_file).st_size > MAX_UPLOAD_SIZE:
LOG.error("The result list to upload is too big.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Write out the maximum uploadable file size too with an exclamation mark at the end of the sentence.

@@ -315,6 +316,11 @@ def main(args):

try:
assemble_zip(args.input, zip_file, client)

if os.stat(zip_file).st_size > MAX_UPLOAD_SIZE:
LOG.error("The result list to upload is too big (max: 1GB).")
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the error message depend on the MAX_UPLOAD_SIZE variable?

Copy link
Contributor

@gyorb gyorb left a comment

Choose a reason for hiding this comment

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

This is just a partial fix for #840 right? This should not close that ticket.
Otherwise LGTM.

@bruntib
Copy link
Contributor Author

bruntib commented Oct 20, 2017

The others items in #840 are already satisfied, So that can be closed. To be exact the 30 sec timeout is not done yet but that shouldn't be relevant, since there is no such a long action during storage. But the timeout can be increased later if needed.

@csordasmarton
Copy link
Contributor

LGTM!

@csordasmarton csordasmarton requested a review from gyorb October 25, 2017 07:41
@csordasmarton csordasmarton merged commit 0983798 into Ericsson:master Oct 25, 2017
@bruntib bruntib deleted the large_upload branch October 25, 2017 12:12
@gyorb
Copy link
Contributor

gyorb commented Oct 27, 2017

I've created a new issue for the missing session improvement. #1072

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants