Skip to content

ref: replace usage of deprecated imp #449

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
brianmay merged 10 commits into from
Jun 10, 2020
Merged

ref: replace usage of deprecated imp #449

brianmay merged 10 commits into from
Jun 10, 2020
+9 −37

Conversation

@joshuarli
Copy link
Contributor

@joshuarli joshuarli commented Jun 3, 2020 •
edited
Loading

sshuttle yells about DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses. It's been deprecated since 3.4, and I see there isn't any intention supporting < 3.5 based on #431.

@joshuarli
Copy link
Contributor Author

joshuarli commented Jun 3, 2020

So 1f25570 and bb52328 are totally broken, but tests don't catch them, which is a bit concerning.

@joshuarli joshuarli mentioned this pull request Jun 4, 2020
Closed
@brianmay
Copy link
Member

brianmay commented Jun 4, 2020

Is this still broken? If so, what problems did you have?

@joshuarli
Copy link
Contributor Author

joshuarli commented Jun 4, 2020

It's the fact that now I'm actually invoking the import mechanism for files that are fundamentally not importable to begin with.

  File "/Users/joshua.li/dev/joshuarli/sshuttle/sshuttle/ssh.py", line 19, in readfile
    module = importlib.import_module(name)
  File "/Users/joshua.li/.local/share/pyenv/versions/3.6.7/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/Users/joshua.li/dev/joshuarli/sshuttle/sshuttle/assembler.py", line 10, in <module>
    verbosity = verbosity  # noqa: F821 must be a previously defined global
NameError: name 'verbosity' is not defined

I noted this in my comments here: https://github.com/sshuttle/sshuttle/pull/449/files#diff-540decd43e8cfa1b8bf41c3b81a8bbb6R5-R8

# Unfortunately, this breaks the new readfile
# since we're actually attempting to import now,
# and this is illegal.
# There's code in ssh.py that prepends a prelude which defines verbosity.

@joshuarli joshuarli changed the title ref: WIP replace usage of deprecated imp ref: replace usage of deprecated imp Jun 9, 2020
@joshuarli joshuarli marked this pull request as ready for review June 9, 2020 07:26
@joshuarli
Copy link
Contributor Author

joshuarli commented Jun 9, 2020 •
edited
Loading

@brianmay I got it working, turns out I just needed to reread the docs for importlib. :) The API is pretty nice/simple to resolve a module path to a file path.

This was the main change since then: 087d07a

brianmay
brianmay approved these changes Jun 10, 2020
View reviewed changes
Copy link
Member

@brianmay brianmay left a comment

Choose a reason for hiding this comment

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

Wow. You reduced the number of lines... That is amazing!

@brianmay brianmay merged commit 0a36eac into sshuttle:master Jun 10, 2020
@joshuarli joshuarli deleted the ref/replace-deprecated-imp branch June 10, 2020 20:58
@jasonboukheir jasonboukheir mentioned this pull request Jun 30, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brianmay brianmay brianmay 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

@joshuarli @brianmay