Skip to content

Commit

Permalink
Better traceback for bad imports in custom path
Browse files Browse the repository at this point in the history
  • Loading branch information
Dimitrios Semitsoglou-Tsiapos committed May 22, 2015
1 parent 00492cd commit 2e9c820
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 7 deletions.
20 changes: 20 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,26 @@ def test_import_string():
pytest.raises(ImportError, utils.import_string, 'cgi.XXXXXXXXXX')


def test_import_string_provides_traceback(tmpdir, monkeypatch):
monkeypatch.syspath_prepend(str(tmpdir))
# Couple of packages
dir_a = tmpdir.mkdir('a')
dir_b = tmpdir.mkdir('b')
# Totally packages, I promise
dir_a.join('__init__.py').write('')
dir_b.join('__init__.py').write('')
# 'aa.a' that depends on 'bb.b', which in turn has a broken import
dir_a.join('aa.py').write('from b import bb')
dir_b.join('bb.py').write('from os import a_typo')

# Do we get all the useful information in the traceback?
with pytest.raises(ImportError) as baz_exc:
utils.import_string('a.aa')
traceback = ''.join((str(line) for line in baz_exc.traceback))
assert 'bb.py\':1' in traceback # a bit different than typical python tb
assert 'from os import a_typo' in traceback


def test_import_string_attribute_error(tmpdir, monkeypatch):
monkeypatch.syspath_prepend(str(tmpdir))
tmpdir.join('foo_test.py').write('from bar_test import value')
Expand Down
8 changes: 1 addition & 7 deletions werkzeug/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,13 +423,7 @@ def import_string(import_name, silent=False):
return sys.modules[import_name]

module_name, obj_name = import_name.rsplit('.', 1)
try:
module = __import__(module_name, None, None, [obj_name])
except ImportError:
# support importing modules not yet set up by the parent module
# (or package for that matter)
module = import_string(module_name)

module = __import__(module_name, globals(), locals(), [obj_name])
try:
return getattr(module, obj_name)
except AttributeError as e:
Expand Down

3 comments on commit 2e9c820

@kaplun
Copy link

@kaplun kaplun commented on 2e9c820 Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dset0x: when are you sending a PR to werkzeug for this?

@kaplun
Copy link

@kaplun kaplun commented on 2e9c820 Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see though why: you are removing a behavior of werkzeug. Maybe you can re-raise the previous exception? Additionally why are you passing globals() and locals() and the original code didn't?

@dset0x
Copy link
Owner

@dset0x dset0x commented on 2e9c820 Aug 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaplun

when are you sending a PR to werkzeug for this?

pallets#735

you are removing a behavior of werkzeug. Maybe you can re-raise the previous exception?

Not from what Werkzeug folks can tell, but can't fully confirm yet. https://github.com/mitsuhiko/werkzeug/pull/735/files

Additionally why are you passing globals() and locals() and the original code didn't?

Because there is no context without them, and without context we fail too early, if memory serves me right.


As far as flask-registry is concerned, we can ship our own method.

Please sign in to comment.