Skip to content

Commit

Permalink
Generate a useful exception on ImportError
Browse files Browse the repository at this point in the history
Rather than leave the user hanging, show something helpful to the user
so they can resolve the situation by installing an additional package.

Left this as a separate function so it's easy to test the exception
generation fucntion doesn't break without needing to do painful testing
contortions. (which should be caught later on by test matrix)
  • Loading branch information
jtratner committed Apr 23, 2024
1 parent 74efbda commit 7cfe9d8
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 10 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
strategy:
matrix:
python-version: [3.7, 3.8, 3.9]
POETRY_EXTRAS: ["-E swift", "-E dxpy", "", "-E swift -E dxpy"]
POETRY_EXTRAS: ["-E swift", "-E dx", "", "-E swift -E dx"]

steps:
- uses: actions/checkout@v2
Expand Down
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Quickstart

::

pip install stor[dxpy, swift]
pip install stor[dx,swift]

``stor`` provides both a CLI and a Python library for manipulating Posix
and OBS with a single, cross-compatible API.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ python-swiftclient = {version = ">=3.6.0", optional = true}

[tool.poetry.extras]
swift = ["python-keystoneclient", "python-swiftclient"]
dxpy = ["dxpy"]
dx = ["dxpy"]

[tool.poetry.dev-dependencies]
flake8 = "^3.7.9"
Expand Down
11 changes: 8 additions & 3 deletions stor/dx.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@

from cached_property import cached_property
from contextlib import contextmanager
import dxpy
from dxpy.exceptions import DXError
from dxpy.exceptions import DXSearchError

from stor import exceptions as stor_exceptions
from stor import Path
Expand All @@ -20,6 +17,14 @@
import stor.settings


try:
import dxpy
from dxpy.exceptions import DXError
from dxpy.exceptions import DXSearchError
except ImportError as e:
raise utils.missing_storage_library_exception("dx", e) from e


logger = logging.getLogger(__name__)
progress_logger = logging.getLogger('%s.progress' % __name__)

Expand Down
12 changes: 8 additions & 4 deletions stor/swift.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,6 @@
import threading

from urllib import parse
from swiftclient import exceptions as swift_exceptions
from swiftclient import service as swift_service
from swiftclient import client as swift_client
from swiftclient.utils import generate_temp_url

from stor import exceptions as stor_exceptions
from stor import is_swift_path
Expand All @@ -57,6 +53,14 @@
from stor.posix import PosixPath
from stor.third_party.backoff import with_backoff

try:
from swiftclient import exceptions as swift_exceptions
from swiftclient import service as swift_service
from swiftclient import client as swift_client
from swiftclient.utils import generate_temp_url
except ImportError as e:
raise utils.missing_storage_library_exception("swift", e) from e


logger = logging.getLogger(__name__)
progress_logger = logging.getLogger('%s.progress' % __name__)
Expand Down
12 changes: 12 additions & 0 deletions stor/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,3 +460,15 @@ def test_path_no_perms(self):
self.mock_copy.side_effect = stor.exceptions.FailedUploadError('foo')
self.assertFalse(utils.is_writeable('s3://stor-test/foo/bar'))
self.assertFalse(self.mock_remove.called)

class TestStorageLibraryException:
def test_generates_appropriate_text(self):
try:
import thisshouldnotexistatall
except ImportError as e:
exc = utils.missing_storage_library_exception("dx", e)
else:
assert False, "this should have raised an import error"
assert "dx" in str(exc)
assert "thisshouldnotexistatall" in str(exc)
assert "To use a 'dx' path, stor needs an additional python library" in str(exc)
21 changes: 21 additions & 0 deletions stor/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import shutil
from subprocess import check_call
import tempfile
import warnings

from stor import exceptions

Expand Down Expand Up @@ -741,3 +742,23 @@ def add_result(self, result):
progress_msg = self.get_progress_message()
if progress_msg: # pragma: no cover
self.logger.log(self.level, progress_msg)

def missing_storage_library_exception(module: str, exc: Exception):
"""Generate a helpful error for user about why their import failed.
Meant to be used as:
try:
...
except Import Error as e:
raise missing_storage_library_exception('dx') from e
"""
return ImportError(
f"{type(exc).__name__}: {exc}\n"
f"To use a '{module}' path, stor needs an additional python library. "
f"Please specify it as an extra in the installation.\n"
f"i.e.,: `pip install stor[{module}]` or `stor[{module}] >= 5` "
f"in requirements.txt or `poetry add stor[{module}]`.\n"
f"Alternatively, change an existing "
f'pyproject.toml file to specify `stor = {{version="5.0", extras = {module}}}`'
)

0 comments on commit 7cfe9d8

Please sign in to comment.