-
Notifications
You must be signed in to change notification settings - Fork 15.6k
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
add support for module import prefix on Python compiler #17286
add support for module import prefix on Python compiler #17286
Conversation
attempt to add prefix only to public imports
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment. This PR is labeled |
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment. This PR is labeled |
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it. This PR was closed and archived because there has been no new activity in the 14 days since the |
Problem
When generating code in monorepos, some generated code is used with a prefix path, see https://rules-proto-grpc.com/en/latest/lang/python.html .
However, when a prefix_path is used, the generated proto import paths are no longer valid unless the module imports are also prefixed similarly.
This adds an option to support adding an import prefix to the printed imports.
Approach
This follows a similar approach already in use in the objectivec compiler.
How did I validate the approach?
I ran some basic manual tests like the following and validated that the module prefix was added to imports:
The diff when comparing with and without the
module_import_prefix=test.prefix
parameter:Remaining TODOs
I couldn't find tests for parameter behavior. plugin_unittest, https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/compiler/python/plugin_unittest.cc doesn't test much of the plugin logic.
I would appreciate some guidance in how to add a test for this feature (or if a test is needed).
Related Issues