Support installing smart_open without AWS dependencies #534
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- don't export s3_iter_bucket at the top-level. We could probably still make this work, but it would be ugly. This is the only breaking change I can see - add suggestive install commands when importing a module that you don't have the deps for
- the tests fail locally without it installed
|
@mpenkov @piskvorky could you provide review/feedback on this? |
- wrap it in a lazy-loaded warning about being deprecated in favor of importing iter_bucket from s3 - add some more tests
- this isn't the prettiest, but it's still better than a generic error that something might be wrong with a link to the readme? - duplicate the schemes list inside of transport.py where the modules are registered. This way if you try to open a URI with that scheme we can show the nice "pip install smart_open[type]" error message. - add tests for transport registration and error propagation
|
I added code to throw the helpful import errors when you call (.env) smart_open > python
>>> from smart_open import open
>>> open("gs://foo")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "smart_open/smart_open/smart_open_lib.py", line 223, in open
binary = _open_binary_stream(uri, binary_mode, transport_params)
File "smart_open/smart_open/smart_open_lib.py", line 401, in _open_binary_stream
submodule = transport.get_transport(scheme)
File "smart_open/smart_open/transport.py", line 86, in get_transport
raise ImportError(_ERRORS[scheme])
ImportError: You are trying to use the GCS functionality of smart_open
but you do not have the correct GCS dependencies installed. Try:
pip install smart_open[gcs]
>>> open("s3://foo")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "smart_open/smart_open/smart_open_lib.py", line 223, in open
binary = _open_binary_stream(uri, binary_mode, transport_params)
File "smart_open/smart_open/smart_open_lib.py", line 401, in _open_binary_stream
submodule = transport.get_transport(scheme)
File "smart_open/smart_open/transport.py", line 86, in get_transport
raise ImportError(_ERRORS[scheme])
ImportError: You are trying to use the S3 functionality of smart_open
but you do not have the correct AWS dependencies installed. Try:
pip install smart_open[aws]
>>> open("azure://foo")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "smart_open/smart_open/smart_open_lib.py", line 223, in open
binary = _open_binary_stream(uri, binary_mode, transport_params)
File "smart_open/smart_open/smart_open_lib.py", line 401, in _open_binary_stream
submodule = transport.get_transport(scheme)
File "smart_open/smart_open/transport.py", line 86, in get_transport
raise ImportError(_ERRORS[scheme])
ImportError: You are trying to use the Azure functionality of smart_open
but you do not have the correct Azure dependencies installed. Try:
pip install smart_open[azure]
|
mpenkov
requested changes
Aug 30, 2020
Co-authored-by: Michael Penkov <m@penkov.dev>
…o fix/install-without-aws
…mart_open into fix/install-without-aws
- DRY up the missing import errors by putting the logic in transport.py - add MISSING_DEPS to transport modules that can be set to true when handling import errors. This allows the submodule to load properly (so we can inspect the scheme/s) even if it's missing required pacakges. - remove double-specified transport schemes (because of MISSING_DEPS)
mpenkov
reviewed
Sep 1, 2020
mpenkov
reviewed
Sep 2, 2020
|
Thanks for the thorough review! I think that I've addressed all your comments now. Let me know if you want any more changes. |
mpenkov
approved these changes
Sep 3, 2020
There was a problem hiding this comment.
Looks good! Thank you for your contribution @justindujardin, and congrats on your first PR to smart_open 🥇
|
@mpenkov any ballpark for when this might ship in a new pypi version? 🙏 |
|
I need to fix #538 first, after that nothing is blocking the release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Title
Support installing smart_open without AWS dependencies
Motivation
Using smart_open with GCS should not require AWS packages to be installed.
Changes
smart_open.openwith an unsupported scheme because of missing dependenciesChecklist
Before you create the PR, please make sure you have: