generated Python gRPC code cannot be imported when the path has dot

[linzhp]

Issue Description

The the path of proto files has dot (e.g., k8s.io/apimachinery/pkg/runtime/generated.proto), the Python modules generated by python_grpc_library cannot be imported

Log Output

  bazel-bin/k8s.io/apimachinary/py_lib_pb/k8s/io/apimachinary/demo_pb2.py
  bazel-bin/k8s.io/apimachinary/py_lib_pb/k8s.io/apimachinary/demo_pb2_grpc.py

Notice the k8s/io vs k8s.io in the paths. The former can be imported, the latter cannot.

rules_proto_grpc Version

5.0.0-alpha3

Bazel Version

7.0.0

OS

Linux

Link to Demo Repo

No response

MODULE.bazel or WORKSPACE Content

module(name="k8s_test")

bazel_dep(name="rules_python", version="0.28.0")
bazel_dep(name="rules_proto_grpc", version="5.0.0")

archive_override(
    module_name = "rules_proto_grpc",
    strip_prefix = "rules_proto_grpc-5.0.0-alpha3",
    urls = ["https://github.com/rules-proto-grpc/rules_proto_grpc/releases/download/5.0.0-alpha3/rules_proto_grpc-5.0.0-alpha3.tar.gz"],
)

bazel_dep(name="rules_proto_grpc_python", version="5.0.0")

archive_override(
    module_name = "rules_proto_grpc_python",
    integrity = "sha256-sQGn6Ph87GEHSkRSQSOauo1nouRJFEmYT5PlSL/EASI=",
    strip_prefix = "rules_proto_grpc_python-5.0.0-alpha3",
    urls = ["https://github.com/rules-proto-grpc/rules_proto_grpc/releases/download/5.0.0-alpha3/rules_proto_grpc_python-5.0.0-alpha3.tar.gz"],
)

BUILD Content

load("@rules_proto//proto:defs.bzl", "proto_library")
load("@rules_proto_grpc_python//:defs.bzl", "python_grpc_library")
load("@rules_python//python:defs.bzl", "py_test")

package(default_visibility = ["//visibility:private"])

# Test that python 3 dependencies protobuf and grpc are importable

proto_library(
    name = "proto_lib",
    srcs = ["demo.proto"],
)

python_grpc_library(
    name = "py_lib",
    protos = [":proto_lib"],
    python_version = "PY3",
)

py_test(
    name = "test",
    srcs = ["test.py"],
    legacy_create_init = False,
    python_version = "PY3",
    deps = [":py_lib"],
)

Proto Content

syntax = "proto3";

message demo {
    bool field = 1;
}

service K8s {
}

Any Other Content

-- MODULE.bazel --
module(name="k8s_test")

bazel_dep(name="rules_python", version="0.28.0")
bazel_dep(name="rules_proto_grpc", version="5.0.0")

archive_override(
    module_name = "rules_proto_grpc",
    strip_prefix = "rules_proto_grpc-5.0.0-alpha3",
    urls = ["https://github.com/rules-proto-grpc/rules_proto_grpc/releases/download/5.0.0-alpha3/rules_proto_grpc-5.0.0-alpha3.tar.gz"],
)

bazel_dep(name="rules_proto_grpc_python", version="5.0.0")

archive_override(
    module_name = "rules_proto_grpc_python",
    integrity = "sha256-sQGn6Ph87GEHSkRSQSOauo1nouRJFEmYT5PlSL/EASI=",
    strip_prefix = "rules_proto_grpc_python-5.0.0-alpha3",
    urls = ["https://github.com/rules-proto-grpc/rules_proto_grpc/releases/download/5.0.0-alpha3/rules_proto_grpc_python-5.0.0-alpha3.tar.gz"],
)
-- WORKSPACE --
-- k8s.io/apimachinary/BUILD.bazel --
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@rules_proto_grpc_python//:defs.bzl", "python_grpc_library")
load("@rules_python//python:defs.bzl", "py_test")

package(default_visibility = ["//visibility:private"])

# Test that python 3 dependencies protobuf and grpc are importable

proto_library(
    name = "proto_lib",
    srcs = ["demo.proto"],
)

python_grpc_library(
    name = "py_lib",
    protos = [":proto_lib"],
    python_version = "PY3",
)

py_test(
    name = "test",
    srcs = ["test.py"],
    legacy_create_init = False,
    python_version = "PY3",
    deps = [":py_lib"],
)
-- k8s.io/apimachinary/demo.proto --
syntax = "proto3";

message demo {
    bool field = 1;
}

service K8s {
}
-- k8s.io/apimachinary/test.py --
import unittest

class TestImports(unittest.TestCase):
    def test_import_pb2(self):
        from k8s.io.apimachinary import demo_pb2
        self.assertGreater(len(dir(demo_pb2)), 0)

    def test_import_pb2(self):
        from k8s.io.apimachinary import demo_pb2_grpc
        self.assertGreater(len(dir(demo_pb2_grpc)), 0)

if __name__ == "__main__":
    unittest.main()

[SDAChess]

I am also looking for an update on this issue. Could it be possible to add the same default as for protobuf generation which generates by default the subdirectories automatically?

[SDAChess]

I don't mind fixing the issue if you give me the go on how you want it fixed.

[linzhp]

@aaliddell what do you think?

[aaliddell]

So I had a dig through the protobuf and grpc code to see where this was coming from when I was reviewing your PR. This is the draft message I was part way through writing and haven't had time recently to come back to sorry:

I've been looking through the Python compiler protoc plugins for protobuf and grpc and this seems like a bug in the grpc plugin, since it makes no attempt to replace the dots to make an importable path here:

https://github.com/grpc/grpc/blob/5174569c4d3352112848f1b86ba259425db939cf/src/compiler/python_generator.cc#L892-L900

The protobuf plugin does however patch dots into slashes:

https://github.com/protocolbuffers/protobuf/blob/a9b006bddd52e289029f16aa77b77e8e0033d9ee/src/google/protobuf/compiler/python/helpers.cc#L79

And the grpc plugin is aware that the protouf plugin does this:

https://github.com/grpc/grpc/blob/5174569c4d3352112848f1b86ba259425db939cf/src/compiler/python_generator_helpers.h#L64-L75

So if anything, the lack of support for dotted paths here is a symptom of a problem elsewhere and the relevant bug is here: https://github.com/grpc/grpc/issues/23978

So regarding the PR, I'm hesitant to put in a fix that makes the plugins behave differently here than they do elsewhere, as that then becomes a divergent implementation.

On that last point: what's the non-Bazel gRPC way of solving this with Python? i.e if you are running grpcio-tools yourself, how would you be expected to fix this? The hesitancy comes having to patch the import paths and introduce some "magic" that doesn't work outside of Bazel, especially since the code is changing the way NO_PREFIX_FLAT is behaving too.

[SDAChess]

I'll try to get a patch merged upstream so that it fixes at least the path problem on gRPC

[aaliddell]

Closed by #310