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

Use absolute import paths in plugin main #66

Closed
wants to merge 3 commits into from

Conversation

aaliddell
Copy link

This change sets the import paths to be absolute in plugin/main.py, due to the following requirement from https://docs.python.org/3/tutorial/modules.html#intra-package-references

Note that relative imports are based on the name of the current module. Since the name of the main module is always "main", modules intended for use as the main module of a Python application must always use absolute imports.

The present solution works when called via the console_scripts in setup.py, but fails elsewhere with SystemError: Parent module '' not loaded, cannot perform relative import.

Also, the method of fetching the version in setup.py has been simplified to avoid needing regex.

Also, another consideration that I can move to its own issue if you like:

  • There appear to be no tests for the plugin presently, as I guess these don't fit so nicely into pytest, but something can probably be worked out. Perhaps by having some demo inputs and outputs to test just the plugin, rather than calling all of protoc, although both would be nice eventually.

@vmagamedov
Copy link
Owner

plugin/main.py is not a "script", this module was made this way intentionally, that's why it doesn't contain code like:

if __name__ == '__main__':
    main()

And I don't see a reasons to change this. If you want to work with the plugin during development, the simplest way is to install the package in editable mode:

$ pip3 install -e .

Then you will be able to invoke it like:

$ cat request.bin | protoc-gen-python_grpc

BTW, absolute imports are not needed if you call this module this way:

$ python -m grpclib.plugin.main

Regex-based version reading in setup.py is actually copied from other projects, it looks weird, but it is proven to be reliable :) Currently grpclib/__init__.py module doesn't contain imports, but this can be changed in the future, and this potentially can brake setup.py script in some way. So I'd say that it is better to stick to the current version.

Testing plugin/main.py is a great idea. I have no reasons for not testing it, just forgot about this. And was lazy to add coverage reports.

@aaliddell
Copy link
Author

Ah I see, I was misusing that. Will close. Thanks

@aaliddell aaliddell closed this Mar 28, 2019
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.

2 participants