Skip to content

Fix gh-3621 broken error handling under PY3 #3622

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Aug 26, 2019
Merged

Fix gh-3621 broken error handling under PY3 #3622

merged 6 commits into from
Aug 26, 2019

Conversation

mih
Copy link
Member

@mih mih commented Aug 26, 2019

Pretty much just moving the affected code (see #3621). Plus replacement of import * usage.

Fingers crossed...

mih and others added 4 commits August 26, 2019 07:23
@mih
BF: Move conditional (back) inside except (fixes dataladgh-3621)
bd975a8 
Mere move, indent, reformat.
@mih
RF: Discontinue evil 'import *'
22392dd
@mih
RF: Adjust import location
9c8a3a8 
Formerly exceptions had been imported blindly into downloaders.base
There seems to be no advantage of having 'import *' at the core, and
then specific imports downstream.
@kyleam
RF: Adjust import locations, round 2
24a99f0 
These locations were missed by the last commit.
@codecov
Copy link

codecov bot commented Aug 26, 2019

Codecov Report

Merging #3622 into master will decrease coverage by 58.07%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3622       +/-   ##
===========================================
- Coverage   82.93%   24.85%   -58.08%     
===========================================
  Files         273      270        -3     
  Lines       35509    35492       -17     
===========================================
- Hits        29450     8823    -20627     
- Misses       6059    26669    +20610
Impacted Files Coverage Δ
datalad/downloaders/s3.py 0% <0%> (-57.37%) ⬇️
datalad/downloaders/http.py 0% <0%> (-61.51%) ⬇️
datalad/downloaders/base.py 0% <0%> (-56.37%) ⬇️
datalad/customremotes/tests/__init__.py 0% <0%> (-100%) ⬇️
datalad/tests/test_protocols.py 0% <0%> (-100%) ⬇️
datalad/tests/test_constraints.py 0% <0%> (-100%) ⬇️
datalad/tests/test_misc.py 0% <0%> (-100%) ⬇️
datalad/distribution/tests/test_clone.py 0% <0%> (-100%) ⬇️
datalad/local/tests/test_subdataset.py 0% <0%> (-100%) ⬇️
datalad/support/tests/test_ansi_colors.py 0% <0%> (-100%) ⬇️
... and 220 more

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 94e335c...24a99f0. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 26, 2019 •
edited
Loading

Codecov Report

Merging #3622 into master will decrease coverage by 58.07%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #3622       +/-   ##
===========================================
- Coverage   82.93%   24.85%   -58.08%     
===========================================
  Files         273      270        -3     
  Lines       35509    35492       -17     
===========================================
- Hits        29450     8823    -20627     
- Misses       6059    26669    +20610
Impacted Files Coverage Δ
datalad/downloaders/s3.py 0% <0%> (-57.37%) ⬇️
datalad/downloaders/http.py 0% <0%> (-61.51%) ⬇️
datalad/downloaders/base.py 0% <0%> (-56.37%) ⬇️
datalad/customremotes/tests/__init__.py 0% <0%> (-100%) ⬇️
datalad/tests/test_protocols.py 0% <0%> (-100%) ⬇️
datalad/tests/test_constraints.py 0% <0%> (-100%) ⬇️
datalad/tests/test_misc.py 0% <0%> (-100%) ⬇️
datalad/distribution/tests/test_clone.py 0% <0%> (-100%) ⬇️
datalad/local/tests/test_subdataset.py 0% <0%> (-100%) ⬇️
datalad/support/tests/test_ansi_colors.py 0% <0%> (-100%) ⬇️
... and 220 more

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 94e335c...8d1a72e. Read the comment docs.

@kyleam
RF: downloaders: Add missing import
876751b 
This one was missed in 22392dd (RF: Discontinue evil 'import *',
2019-08-26).
@mih
Copy link
Member Author

mih commented Aug 26, 2019

@kyleam Thx, I almost had it edited myself, almost... ;-)

@kyleam
Copy link
Contributor

kyleam commented Aug 26, 2019

Thx, I almost had it edited myself, almost... ;-)

No problem. Those were caught by the tests, but I spotted one more. Pushing shortly.

@kyleam
RF: downloaders: Remove unneeded condition
8d1a72e 
As of bd975a8, this code is under the AccessDeniedError exception
handling, so the access_denied flag is no longer needed.
@kyleam
Copy link
Contributor

kyleam commented Aug 26, 2019 •
edited
Loading

Some Travis runs failed with network issues. Restarted.

Edit: Actually, I guess not network issues:

+sudo apt-get update -qq
E: Failed to fetch http://apt.postgresql.org/pub/repos/apt/dists/xenial-pgdg/main/binary-i386/Packages  Writing more data than expected (788347 > 787357)

The restarted runs have failed as well. Given that AppVeyor and many of the main Travis runs passed, I think these setup failures shouldn't hold up this PR.

@mih mih merged commit c9abcb0 into datalad:master Aug 26, 2019
@mih mih deleted the bf-3621 branch August 26, 2019 15:04
@mih
Copy link
Member Author

mih commented Aug 26, 2019

Thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyleam kyleam kyleam approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mih @kyleam