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

Fix not catching errors caused by invalid files #410

Merged
merged 5 commits into from Mar 30, 2018

Conversation

@libre-man
Copy link
Collaborator

commented Mar 27, 2018

This closes #384.

@codecov

This comment has been minimized.

Copy link

commented Mar 27, 2018

Codecov Report

Merging #410 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #410      +/-   ##
==========================================
+ Coverage   99.03%   99.04%   +<.01%     
==========================================
  Files          29       29              
  Lines        3326     3337      +11     
==========================================
+ Hits         3294     3305      +11     
  Misses         32       32
Impacted Files Coverage Δ
psef/ignore.py 100% <100%> (ø) ⬆️
psef/files.py 99.5% <100%> (+0.01%) ⬆️
psef/errors.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4514b46...a6f5806. Read the comment docs.

except (tarfile.ReadError, zipfile.BadZipFile):
raise APIException(
'The given archive could not be extracted',
"The given archive doesn't seem to be an archive",

This comment has been minimized.

Copy link
@olmokramer

olmokramer Mar 30, 2018

Collaborator

Inconsistent use of quotes

This comment has been minimized.

Copy link
@libre-man

libre-man Mar 30, 2018

Author Collaborator

NOO!

)
except InvalidFile as e:
raise APIException(
str(e),

This comment has been minimized.

Copy link
@olmokramer

olmokramer Mar 30, 2018

Collaborator

Here you put the str(e) first, then a custom message, but below you have the custom message first and then the str(e)... Is this intentional?

This comment has been minimized.

Copy link
@libre-man

libre-man Mar 30, 2018

Author Collaborator

Yes, this as we control this exception message so we can make sure there is useful information for the user in it.

@ghost ghost assigned olmokramer Mar 30, 2018

olmokramer and others added 3 commits Mar 30, 2018

@libre-man libre-man merged commit 2476bb4 into master Mar 30, 2018

5 checks passed

codecov/patch 100% of diff hit (target 99.03%)
Details
codecov/project 99.04% (+<.01%) compared to 4514b46
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 99.041%
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details

@ghost ghost removed the in progress label Mar 30, 2018

@libre-man libre-man deleted the fix-invalid-archive-errors branch Mar 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.