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

Update hook-passlib.py #39

Merged
merged 3 commits into from
Aug 28, 2020
Merged

Update hook-passlib.py #39

merged 3 commits into from
Aug 28, 2020

Conversation

mhils
Copy link
Contributor

@mhils mhils commented Aug 27, 2020

Thank you @Legorooj and other for setting this repo up here! 😃 We've been maintaining some custom third-party hooks for @mitmproxy for a while, which I'd be happy to contribute here.

To start things off, passlib is missing a hidden import. Without configparser, importing passlib yields the following traceback:

    import passlib.apache
  File "lib/python3.7/site-packages/PyInstaller/loader/pyimod03_importers.py", line 493, in exec_module
    exec(bytecode, module.__dict__)
  File "passlib/apache.py", line 14, in <module>
  File "lib/python3.7/site-packages/PyInstaller/loader/pyimod03_importers.py", line 493, in exec_module
    exec(bytecode, module.__dict__)
  File "passlib/context.py", line 21, in <module>
  File "passlib/utils/compat/__init__.py", line 417, in __getattr__
  File "passlib/utils/compat/__init__.py", line 379, in _import_object
ModuleNotFoundError: No module named 'configparser'

Without `configparser`, importing passlib yields the following traceback:
```
    import passlib.apache
  File "lib/python3.7/site-packages/PyInstaller/loader/pyimod03_importers.py", line 493, in exec_module
    exec(bytecode, module.__dict__)
  File "passlib/apache.py", line 14, in <module>
  File "lib/python3.7/site-packages/PyInstaller/loader/pyimod03_importers.py", line 493, in exec_module
    exec(bytecode, module.__dict__)
  File "passlib/context.py", line 21, in <module>
  File "passlib/utils/compat/__init__.py", line 417, in __getattr__
  File "passlib/utils/compat/__init__.py", line 379, in _import_object
ModuleNotFoundError: No module named 'configparser'
```
@mhils mhils requested review from a team and bwoodsend and removed request for a team August 27, 2020 08:49
@bwoodsend
Copy link
Member

We've been maintaining some custom third-party hooks for @mitmproxy for a while, which I'd be happy to contribute here.

Yes please 😄.

Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

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

This needs a changelog entry. See the news README.

Optionally, it would be good if we could have a test for this too...

@mhils
Copy link
Contributor Author

mhils commented Aug 27, 2020

Would you like me to open separate PRs for each hook or update everything here? There will definitely be some merge conflicts in the former case due to tests and requirements being added at the same location.

@bwoodsend
Copy link
Member

Definitely separate PRs please.

news/38.update.rst Outdated Show resolved Hide resolved
Copy link
Member

@bwoodsend bwoodsend left a comment

Choose a reason for hiding this comment

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

I'll let CI finish, then we're probably good to go.

@bwoodsend
Copy link
Member

Well tests pass so looks good. Normally we have @Legorooj to give it a final check but he's away. @amifunny Could you give this a once over then merge it if you think it's OK?

Copy link
Contributor

@amifunny amifunny left a comment

Choose a reason for hiding this comment

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

Everything is ok. @bwoodsend Should merge now.

@bwoodsend
Copy link
Member

@amifunny Go for it.

@amifunny
Copy link
Contributor

@amifunny Go for it.

@bwoodsend You want me to merge? I don't have commit rights.

@Legorooj
Copy link
Member

@bwoodsend I haven't given @amifunny write permission here yet. However, I suggest you get him to review PRs first, before you do, so he can practice.

@Legorooj Legorooj merged commit f70a786 into pyinstaller:master Aug 28, 2020
@mhils mhils deleted the patch-1 branch August 28, 2020 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants