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

Fix relative includes in case the files are next to each other #27

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

jankatins
Copy link
Contributor

@jankatins jankatins commented Nov 1, 2023

The code had a bug that in case all folders were shared (=no path elements to add to the module name), the code ended up with a . in front of the name before adding relative dots and therefore had one dot too many.

The code had a bug that in case all folders were shared (=no path elements to add to the module name= , the code ended up with a . in front of the name before adding relative dots.

module_name: str = (
".".join(message_path_list[index + 1 : -1])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if message_path_list[index + 1 : -1] returned an empty list, the module_name at this point started with a dot due to the + "." in the next line.

@so1n
Copy link
Owner

so1n commented Nov 2, 2023

Thanks for your PR, it will make the code more robust. But I can't find a scenario that triggers this problem. Can you provide the Protobuf file and directory structure that triggers this problem?

@so1n so1n merged commit 857c3f9 into so1n:master Nov 2, 2023
4 checks passed
@jankatins
Copy link
Contributor Author

jankatins commented Nov 2, 2023

My code looked like this:

[23:21:45] λ  tree build/proto
build/proto
├── buf.gen.yaml
└── example_model
    ├── __init__.py
    └── v1
        ├── __init__.py
        ├── classes.proto
        ├── classes_p2p.py
        ├── classes_pb2.py
        ├── classes_pb2.pyi
        ├── classes_pb2_grpc.py
        ├── enums.proto
        ├── enums_p2p.py
        ├── enums_pb2.py
        ├── enums_pb2.pyi
        └── enums_pb2_grpc.py

With classes.proto containing:

syntax = "proto3";

package example_model.v1;

import "example_model/v1/enums.proto";

[...]

The code which generated the above python classes was:

PROTO_LINT_DOCKER_BUF_VERSION = 1.27.1
PROTO_LINT_DOCKER_YOHEIMUTA_VERSION = 0.46.2
PROTO_LINT_BUF_BIN ?= docker run --rm --volume ".:/workspace" --workdir /workspace bufbuild/buf:$(PROTO_LINT_DOCKER_BUF_VERSION)
PROTO_LINT_YOHEIMUTA_BIN ?= docker run --rm --volume ".:/workspace" --workdir /workspace yoheimuta/protolint:$(PROTO_LINT_DOCKER_YOHEIMUTA_VERSION)
_proto_folder = ./build/proto
test-and-lint-proto-export
	-rm -rf $(_proto_folder)/
	# generate the classes.proto and enum.proto files
	$(PYTHON_POETRY_RUN) export-proto ...
	# https://github.com/yoheimuta/protolint
	$(PROTO_LINT_YOHEIMUTA_BIN) lint $(_proto_folder)/
	# https://github.com/bufbuild/buf
	$(PROTO_LINT_BUF_BIN) lint $(_proto_folder)/ --config buf.yaml
	cp buf.gen.yaml $(_proto_folder)
	cd $(_proto_folder) && $(PROTO_LINT_BUF_BIN) generate .
	# not in buf generate because we run in docker and https://github.com/so1n/protobuf_to_pydantic/issues/26
	cd $(_proto_folder) && poetry run python -m grpc_tools.protoc -I. --protobuf-to-pydantic_out=. example_model/v1/classes.proto
	cd $(_proto_folder) && poetry run python -m grpc_tools.protoc -I. --protobuf-to-pydantic_out=. example_model/v1/enums.proto
	# Make proper python packages
	cd $(_proto_folder) && touch example_model/__init__.py && touch example_model/v1/__init__.py
	# This currently fails because protobuf-to-pydantic has a bug:
	# https://github.com/so1n/protobuf_to_pydantic/pull/27
	-cd $(_proto_folder) && python -c "from example_model.v1 import classes_p2p, classes_pb2; print('Yay, proto could be exported to valid python files')"

buf.gen.yaml contains:

version: v1
plugins:
  - plugin: buf.build/protocolbuffers/python:v23.2
    out: .
  - plugin: buf.build/grpc/python:v1.56.0
    out: .
  - plugin: buf.build/community/nipunn1313-mypy:v3.4.0
    out: .

@jankatins
Copy link
Contributor Author

@so1n Any chance to get a .4 release with this in it?

@so1n
Copy link
Owner

so1n commented Nov 3, 2023

@jankatins Version 0.2.0.4 has been released, it only contains the current pr. Relevant unit tests will be added later in version 0.2.1.

@jankatins
Copy link
Contributor Author

Thanks a lot!

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