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

Error nicely if no file size inferred #5231

Merged
merged 3 commits into from Aug 7, 2019

Conversation

@jcrist
Copy link
Member

commented Aug 6, 2019

Error nicely for filesystems that fail to provide file size information.

This can happen with e.g. the http filesystem.

I wasn't sure how to test this, it will currently fail for the http filesystem until fsspec is released with the patch in intake/filesystem_spec#93. I'm fine without a test for now if others are.

Fixes #5222.

Error nicely if no file size inferred
Error nicely for filesystems that fail to provide file size information.

This can happen with e.g. the http filesystem.
@jcrist

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

@TomAugspurger

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

I'd prefer it this had a test that is xfailed for fsspecs without intake/filesystem_spec#93. Otherwise I worry that the test never ends up getting written.

@martindurant

This comment has been minimized.

Copy link
Member

commented Aug 6, 2019

Yes agree, can add a new test which is skipped unless we have fsspec>=0.4.2, which I can release sometime soon. Note intake/filesystem_spec#97 , a place to add more thorough tests of HTTP functionality.

@TomAugspurger TomAugspurger added the io label Aug 6, 2019

@jcrist

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

Sure, test added.

dask/bytes/core.py Outdated Show resolved Hide resolved
@martindurant

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

+1

@jcrist jcrist merged commit 98204c5 into dask:master Aug 7, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jcrist jcrist deleted the jcrist:error-nicely-on-no-file-size branch Aug 7, 2019

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